Merge pull request #135 from acelaya/feature/shlink-client-improvements

Removed duplicated code when building ShlinkApiClient
This commit is contained in:
Alejandro Celaya 2019-04-21 11:37:14 +02:00 committed by GitHub
commit e5ef2eb5c6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 60 additions and 52 deletions

View file

@ -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;

View file

@ -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
&nbsp;<FontAwesomeIcon icon={menuIcon} />&nbsp;
</DropdownToggle>
<DropdownMenu right>
<DropdownItem tag={Link} to={`/server/${id}/short-code/${shortUrl.shortCode}/visits`}>
<DropdownItem tag={Link} to={`/server/${selectedServer ? selectedServer.id : ''}/short-code/${shortUrl.shortCode}/visits`}>
<FontAwesomeIcon icon={pieChartIcon} /> &nbsp;Visit stats
</DropdownItem>

View file

@ -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) {

View file

@ -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);

View file

@ -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) {

View file

@ -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);

View file

@ -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 });

View file

@ -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) {

View file

@ -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,

View file

@ -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);

View file

@ -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);

View file

@ -51,3 +51,5 @@ export const useToggle = (initialValue = false) => {
return [ flag, () => setFlag(!flag) ];
};
export const wait = (seconds) => new Promise((resolve) => setTimeout(resolve, seconds));

View file

@ -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) {

View file

@ -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;

View file

@ -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);