Skip to content

Commit 72dd2bd

Browse files
acelayaacelaya
authored
Merge pull request #282 from acelaya-forks/feature/mercure-dup-code
Feature/mercure dup code
2 parents 05e3e87 + 54733ea commit 72dd2bd

File tree

16 files changed

+58
-71
lines changed

16 files changed

+58
-71
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1212

1313
#### Changed
1414

15-
* *Nothing*
15+
* [#254](https://github.com/shlinkio/shlink-web-client/issues/254) Reduced duplication on code to handle mercure topics binding.
1616

1717
#### Deprecated
1818

‎src/mercure/helpers/index.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1+
import { useEffect } from 'react';
12
import { EventSourcePolyfill as EventSource } from 'event-source-polyfill';
23

3-
export const bindToMercureTopic = (mercureInfo, realTimeUpdates, topic, onMessage, onTokenExpired) => () => {
4-
const { enabled } = realTimeUpdates;
4+
export const bindToMercureTopic = (mercureInfo, topic, onMessage, onTokenExpired) => () => {
55
const { mercureHubUrl, token, loading, error } = mercureInfo;
66

7-
if (!enabled || loading || error) {
7+
if (loading || error) {
88
return undefined;
99
}
1010

@@ -22,3 +22,7 @@ export const bindToMercureTopic = (mercureInfo, realTimeUpdates, topic, onMessag
2222

2323
return () => es.close();
2424
};
25+
26+
export const useMercureTopicBinding = (mercureInfo, topic, onMessage, onTokenExpired) => {
27+
useEffect(bindToMercureTopic(mercureInfo, topic, onMessage, onTokenExpired), [ mercureInfo ]);
28+
};

‎src/mercure/reducers/mercureInfo.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,16 @@ export default handleActions({
2929

3030
export const loadMercureInfo = (buildShlinkApiClient) => () => async (dispatch, getState) => {
3131
dispatch({ type: GET_MERCURE_INFO_START });
32+
33+
const { settings } = getState();
3234
const { mercureInfo } = buildShlinkApiClient(getState);
3335

36+
if (!settings.realTimeUpdates.enabled) {
37+
dispatch({ type: GET_MERCURE_INFO_ERROR });
38+
39+
return;
40+
}
41+
3442
try {
3543
const result = await mercureInfo();
3644

‎src/short-urls/ShortUrlsList.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ import { serverType } from '../servers/prop-types';
88
import SortingDropdown from '../utils/SortingDropdown';
99
import { determineOrderDir } from '../utils/utils';
1010
import { MercureInfoType } from '../mercure/reducers/mercureInfo';
11-
import { bindToMercureTopic } from '../mercure/helpers';
12-
import { SettingsType } from '../settings/reducers/settings';
11+
import { useMercureTopicBinding } from '../mercure/helpers';
1312
import { shortUrlType } from './reducers/shortUrlsList';
1413
import { shortUrlsListParamsType } from './reducers/shortUrlsListParams';
1514
import './ShortUrlsList.scss';
@@ -34,7 +33,6 @@ const propTypes = {
3433
createNewVisit: PropTypes.func,
3534
loadMercureInfo: PropTypes.func,
3635
mercureInfo: MercureInfoType,
37-
settings: SettingsType,
3836
};
3937

4038
// FIXME Replace with typescript: (ShortUrlsRow component)
@@ -52,7 +50,6 @@ const ShortUrlsList = (ShortUrlsRow) => {
5250
createNewVisit,
5351
loadMercureInfo,
5452
mercureInfo,
55-
settings: { realTimeUpdates },
5653
}) => {
5754
const { orderBy } = shortUrlsListParams;
5855
const [ order, setOrder ] = useState({
@@ -119,10 +116,7 @@ const ShortUrlsList = (ShortUrlsRow) => {
119116

120117
return resetShortUrlParams;
121118
}, []);
122-
useEffect(
123-
bindToMercureTopic(mercureInfo, realTimeUpdates, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo),
124-
[ mercureInfo ]
125-
);
119+
useMercureTopicBinding(mercureInfo, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo);
126120

127121
return (
128122
<React.Fragment>

‎src/short-urls/services/provideServices.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const provideServices = (bottle, connect) => {
3131

3232
bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsRow');
3333
bottle.decorator('ShortUrlsList', connect(
34-
[ 'selectedServer', 'shortUrlsListParams', 'mercureInfo', 'settings' ],
34+
[ 'selectedServer', 'shortUrlsListParams', 'mercureInfo' ],
3535
[ 'listShortUrls', 'resetShortUrlParams', 'createNewVisit', 'loadMercureInfo' ]
3636
));
3737

‎src/tags/TagsList.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import Message from '../utils/Message';
55
import SearchField from '../utils/SearchField';
66
import { serverType } from '../servers/prop-types';
77
import { MercureInfoType } from '../mercure/reducers/mercureInfo';
8-
import { SettingsType } from '../settings/reducers/settings';
9-
import { bindToMercureTopic } from '../mercure/helpers';
8+
import { useMercureTopicBinding } from '../mercure/helpers';
109
import { TagsListType } from './reducers/tagsList';
1110

1211
const { ceil } = Math;
@@ -20,23 +19,18 @@ const propTypes = {
2019
createNewVisit: PropTypes.func,
2120
loadMercureInfo: PropTypes.func,
2221
mercureInfo: MercureInfoType,
23-
settings: SettingsType,
2422
};
2523

2624
const TagsList = (TagCard) => {
2725
const TagListComp = (
28-
{ filterTags, forceListTags, tagsList, selectedServer, createNewVisit, loadMercureInfo, mercureInfo, settings }
26+
{ filterTags, forceListTags, tagsList, selectedServer, createNewVisit, loadMercureInfo, mercureInfo }
2927
) => {
30-
const { realTimeUpdates } = settings;
3128
const [ displayedTag, setDisplayedTag ] = useState();
3229

3330
useEffect(() => {
3431
forceListTags();
3532
}, []);
36-
useEffect(
37-
bindToMercureTopic(mercureInfo, realTimeUpdates, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo),
38-
[ mercureInfo ]
39-
);
33+
useMercureTopicBinding(mercureInfo, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo);
4034

4135
const renderContent = () => {
4236
if (tagsList.loading) {

‎src/tags/services/provideServices.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const provideServices = (bottle, connect) => {
2929

3030
bottle.serviceFactory('TagsList', TagsList, 'TagCard');
3131
bottle.decorator('TagsList', connect(
32-
[ 'tagsList', 'selectedServer', 'mercureInfo', 'settings' ],
32+
[ 'tagsList', 'selectedServer', 'mercureInfo' ],
3333
[ 'forceListTags', 'filterTags', 'createNewVisit', 'loadMercureInfo' ]
3434
));
3535

‎src/visits/ShortUrlVisits.js

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import React, { useEffect } from 'react';
22
import PropTypes from 'prop-types';
33
import qs from 'qs';
44
import { MercureInfoType } from '../mercure/reducers/mercureInfo';
5-
import { bindToMercureTopic } from '../mercure/helpers';
6-
import { SettingsType } from '../settings/reducers/settings';
5+
import { useMercureTopicBinding } from '../mercure/helpers';
76
import { shortUrlVisitsType } from './reducers/shortUrlVisits';
87
import ShortUrlVisitsHeader from './ShortUrlVisitsHeader';
98
import { shortUrlDetailType } from './reducers/shortUrlDetail';
@@ -26,7 +25,6 @@ const propTypes = {
2625
createNewVisit: PropTypes.func,
2726
loadMercureInfo: PropTypes.func,
2827
mercureInfo: MercureInfoType,
29-
settings: SettingsType,
3028
};
3129

3230
const ShortUrlVisits = (VisitsStats) => {
@@ -42,7 +40,6 @@ const ShortUrlVisits = (VisitsStats) => {
4240
createNewVisit,
4341
loadMercureInfo,
4442
mercureInfo,
45-
settings: { realTimeUpdates },
4643
}) => {
4744
const { params } = match;
4845
const { shortCode } = params;
@@ -54,16 +51,7 @@ const ShortUrlVisits = (VisitsStats) => {
5451
useEffect(() => {
5552
getShortUrlDetail(shortCode, domain);
5653
}, []);
57-
useEffect(
58-
bindToMercureTopic(
59-
mercureInfo,
60-
realTimeUpdates,
61-
`https://shlink.io/new-visit/${shortCode}`,
62-
createNewVisit,
63-
loadMercureInfo
64-
),
65-
[ mercureInfo ],
66-
);
54+
useMercureTopicBinding(mercureInfo, `https://shlink.io/new-visit/${shortCode}`, createNewVisit, loadMercureInfo);
6755

6856
return (
6957
<VisitsStats getVisits={loadVisits} cancelGetVisits={cancelGetShortUrlVisits} visitsInfo={shortUrlVisits}>

‎src/visits/TagVisits.js

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import React, { useEffect } from 'react';
1+
import React from 'react';
22
import PropTypes from 'prop-types';
33
import { MercureInfoType } from '../mercure/reducers/mercureInfo';
4-
import { SettingsType } from '../settings/reducers/settings';
5-
import { bindToMercureTopic } from '../mercure/helpers';
4+
import { useMercureTopicBinding } from '../mercure/helpers';
65
import { TagVisitsType } from './reducers/tagVisits';
76
import TagVisitsHeader from './TagVisitsHeader';
87

@@ -19,7 +18,6 @@ const propTypes = {
1918
createNewVisit: PropTypes.func,
2019
loadMercureInfo: PropTypes.func,
2120
mercureInfo: MercureInfoType,
22-
settings: SettingsType,
2321
};
2422

2523
const TagVisits = (VisitsStats, colorGenerator) => {
@@ -32,22 +30,12 @@ const TagVisits = (VisitsStats, colorGenerator) => {
3230
createNewVisit,
3331
loadMercureInfo,
3432
mercureInfo,
35-
settings: { realTimeUpdates },
3633
}) => {
3734
const { params } = match;
3835
const { tag } = params;
3936
const loadVisits = (dates) => getTagVisits(tag, dates);
4037

41-
useEffect(
42-
bindToMercureTopic(
43-
mercureInfo,
44-
realTimeUpdates,
45-
'https://shlink.io/new-visit',
46-
createNewVisit,
47-
loadMercureInfo
48-
),
49-
[ mercureInfo ],
50-
);
38+
useMercureTopicBinding(mercureInfo, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo);
5139

5240
return (
5341
<VisitsStats getVisits={loadVisits} cancelGetVisits={cancelGetTagVisits} visitsInfo={tagVisits}>

‎src/visits/services/provideServices.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ const provideServices = (bottle, connect) => {
1616
bottle.serviceFactory('VisitsStats', VisitsStats, 'VisitsParser', 'OpenMapModalBtn');
1717
bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsStats');
1818
bottle.decorator('ShortUrlVisits', connect(
19-
[ 'shortUrlVisits', 'shortUrlDetail', 'mercureInfo', 'settings' ],
19+
[ 'shortUrlVisits', 'shortUrlDetail', 'mercureInfo' ],
2020
[ 'getShortUrlVisits', 'getShortUrlDetail', 'cancelGetShortUrlVisits', 'createNewVisit', 'loadMercureInfo' ]
2121
));
2222
bottle.serviceFactory('TagVisits', TagVisits, 'VisitsStats', 'ColorGenerator');
2323
bottle.decorator('TagVisits', connect(
24-
[ 'tagVisits', 'mercureInfo', 'settings' ],
24+
[ 'tagVisits', 'mercureInfo' ],
2525
[ 'getTagVisits', 'cancelGetTagVisits', 'createNewVisit', 'loadMercureInfo' ]
2626
));
2727

‎test/mercure/helpers/index.test.js

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,11 @@ describe('helpers', () => {
1111
const onTokenExpired = jest.fn();
1212

1313
it.each([
14-
[{ loading: true, error: false }, { enabled: true }],
15-
[{ loading: false, error: true }, { enabled: true }],
16-
[{ loading: true, error: true }, { enabled: true }],
17-
[{ loading: false, error: false }, { enabled: false }],
18-
])('does not bind an EventSource when disabled, loading or error', (mercureInfo, realTimeUpdates) => {
19-
bindToMercureTopic(mercureInfo, realTimeUpdates)();
14+
[{ loading: true, error: false }],
15+
[{ loading: false, error: true }],
16+
[{ loading: true, error: true }],
17+
])('does not bind an EventSource when loading or error', (mercureInfo) => {
18+
bindToMercureTopic(mercureInfo)();
2019

2120
expect(EventSource).not.toHaveBeenCalled();
2221
expect(onMessage).not.toHaveBeenCalled();
@@ -36,7 +35,7 @@ describe('helpers', () => {
3635
error: false,
3736
mercureHubUrl,
3837
token,
39-
}, { enabled: true }, topic, onMessage, onTokenExpired)();
38+
}, topic, onMessage, onTokenExpired)();
4039

4140
expect(EventSource).toHaveBeenCalledWith(hubUrl, {
4241
headers: {

‎test/mercure/reducers/mercureInfo.test.js

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,31 @@ describe('mercureInfoReducer', () => {
4040
mercureInfo: jest.fn(() => result),
4141
});
4242
const dispatch = jest.fn();
43-
const getState = () => ({});
43+
const createGetStateMock = (enabled) => jest.fn(() => ({
44+
settings: {
45+
realTimeUpdates: { enabled },
46+
},
47+
}));
4448

4549
afterEach(jest.resetAllMocks);
4650

51+
it('dispatches error when real time updates are disabled', async () => {
52+
const apiClientMock = createApiClientMock(Promise.resolve(mercureInfo));
53+
const getState = createGetStateMock(false);
54+
55+
await loadMercureInfo(() => apiClientMock)()(dispatch, getState);
56+
57+
expect(apiClientMock.mercureInfo).not.toHaveBeenCalled();
58+
expect(dispatch).toHaveBeenCalledTimes(2);
59+
expect(dispatch).toHaveBeenNthCalledWith(1, { type: GET_MERCURE_INFO_START });
60+
expect(dispatch).toHaveBeenNthCalledWith(2, { type: GET_MERCURE_INFO_ERROR });
61+
});
62+
4763
it('calls API on success', async () => {
4864
const apiClientMock = createApiClientMock(Promise.resolve(mercureInfo));
65+
const getState = createGetStateMock(true);
4966

50-
await loadMercureInfo(() => apiClientMock)()(dispatch, getState());
67+
await loadMercureInfo(() => apiClientMock)()(dispatch, getState);
5168

5269
expect(apiClientMock.mercureInfo).toHaveBeenCalledTimes(1);
5370
expect(dispatch).toHaveBeenCalledTimes(2);
@@ -58,8 +75,9 @@ describe('mercureInfoReducer', () => {
5875
it('throws error on failure', async () => {
5976
const error = 'Error';
6077
const apiClientMock = createApiClientMock(Promise.reject(error));
78+
const getState = createGetStateMock(true);
6179

62-
await loadMercureInfo(() => apiClientMock)()(dispatch, getState());
80+
await loadMercureInfo(() => apiClientMock)()(dispatch, getState);
6381

6482
expect(apiClientMock.mercureInfo).toHaveBeenCalledTimes(1);
6583
expect(dispatch).toHaveBeenCalledTimes(2);

‎test/short-urls/ShortUrlsList.test.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ describe('<ShortUrlsList />', () => {
99
const ShortUrlsRow = () => '';
1010
const listShortUrlsMock = jest.fn();
1111
const resetShortUrlParamsMock = jest.fn();
12-
const realTimeUpdates = { enabled: true };
1312

1413
const ShortUrlsList = shortUrlsListCreator(ShortUrlsRow);
1514

@@ -38,7 +37,6 @@ describe('<ShortUrlsList />', () => {
3837
]
3938
}
4039
mercureInfo={{ loading: true }}
41-
settings={{ realTimeUpdates }}
4240
/>
4341
);
4442
});

‎test/tags/TagsList.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ describe('<TagsList />', () => {
1515
const TagsList = createTagsList(TagCard);
1616

1717
wrapper = shallow(
18-
<TagsList forceListTags={identity} filterTags={filterTags} match={{ params }} tagsList={tagsList} settings={{}} />
18+
<TagsList forceListTags={identity} filterTags={filterTags} match={{ params }} tagsList={tagsList} />
1919
);
2020

2121
return wrapper;

‎test/visits/ShortUrlVisits.test.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ describe('<ShortUrlVisits />', () => {
1414
const history = {
1515
goBack: jest.fn(),
1616
};
17-
const realTimeUpdates = { enabled: true };
1817
const VisitsStats = jest.fn();
1918

2019
beforeEach(() => {
@@ -31,7 +30,6 @@ describe('<ShortUrlVisits />', () => {
3130
shortUrlDetail={{}}
3231
cancelGetShortUrlVisits={identity}
3332
matchMedia={() => ({ matches: false })}
34-
settings={{ realTimeUpdates }}
3533
/>
3634
);
3735
});

‎test/visits/TagVisits.test.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ describe('<TagVisits />', () => {
1313
const history = {
1414
goBack: jest.fn(),
1515
};
16-
const realTimeUpdates = { enabled: true };
1716
const VisitsStats = jest.fn();
1817

1918
beforeEach(() => {
@@ -26,7 +25,6 @@ describe('<TagVisits />', () => {
2625
history={history}
2726
tagVisits={{ loading: true, visits: [] }}
2827
cancelGetTagVisits={identity}
29-
settings={{ realTimeUpdates }}
3028
/>
3129
);
3230
});

0 commit comments

Comments
 (0)