From dc672bf0f0c4261a65713dc929d448d91e94c85c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:07:55 +0100 Subject: [PATCH 1/9] Ensured domain is passed when loading visits for a short URL on a specific domain --- src/short-urls/helpers/ShortUrlVisitsCount.js | 20 +++++++---- src/short-urls/helpers/ShortUrlsRow.js | 6 +++- src/short-urls/helpers/ShortUrlsRowMenu.js | 4 +-- src/short-urls/helpers/VisitStatsLink.js | 29 ++++++++++++++++ src/short-urls/reducers/shortUrlsList.js | 1 + src/utils/services/ShlinkApiClient.js | 21 ++++++------ src/visits/ShortUrlVisits.js | 12 +++++-- src/visits/VisitsHeader.js | 2 +- src/visits/reducers/shortUrlVisits.js | 6 ++-- .../helpers/ShortUrlVisitsCount.test.js | 10 +++--- .../helpers/ShortUrlsRowMenu.test.js | 2 ++ test/utils/services/ShlinkApiClient.test.js | 34 ++++++++++++------- test/visits/ShortUrlVisits.test.js | 4 ++- test/visits/VisitsHeader.test.js | 2 +- 14 files changed, 108 insertions(+), 45 deletions(-) create mode 100644 src/short-urls/helpers/VisitStatsLink.js diff --git a/src/short-urls/helpers/ShortUrlVisitsCount.js b/src/short-urls/helpers/ShortUrlVisitsCount.js index 6a5c9359..a268c9b5 100644 --- a/src/short-urls/helpers/ShortUrlVisitsCount.js +++ b/src/short-urls/helpers/ShortUrlVisitsCount.js @@ -3,25 +3,33 @@ import PropTypes from 'prop-types'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faInfoCircle as infoIcon } from '@fortawesome/free-solid-svg-icons'; import { UncontrolledTooltip } from 'reactstrap'; -import { shortUrlMetaType } from '../reducers/shortUrlMeta'; +import { serverType } from '../../servers/prop-types'; +import { shortUrlType } from '../reducers/shortUrlsList'; import './ShortUrlVisitsCount.scss'; +import VisitStatsLink from './VisitStatsLink'; const propTypes = { visitsCount: PropTypes.number.isRequired, - meta: shortUrlMetaType, + shortUrl: shortUrlType, + selectedServer: serverType, }; -const ShortUrlVisitsCount = ({ visitsCount, meta }) => { - const maxVisits = meta && meta.maxVisits; +const ShortUrlVisitsCount = ({ visitsCount, shortUrl, selectedServer }) => { + const maxVisits = shortUrl && shortUrl.meta && shortUrl.meta.maxVisits; + const visitsLink = ( + + {visitsCount} + + ); if (!maxVisits) { - return {visitsCount}; + return visitsLink; } return ( - {visitsCount} + {visitsLink} {' '}/ {maxVisits}{' '} diff --git a/src/short-urls/helpers/ShortUrlsRow.js b/src/short-urls/helpers/ShortUrlsRow.js index 43d4205f..a0cbf4a8 100644 --- a/src/short-urls/helpers/ShortUrlsRow.js +++ b/src/short-urls/helpers/ShortUrlsRow.js @@ -58,7 +58,11 @@ const ShortUrlsRow = ( {this.renderTags(shortUrl.tags)} - +   - + Visit stats diff --git a/src/short-urls/helpers/VisitStatsLink.js b/src/short-urls/helpers/VisitStatsLink.js new file mode 100644 index 00000000..f500e580 --- /dev/null +++ b/src/short-urls/helpers/VisitStatsLink.js @@ -0,0 +1,29 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Link } from 'react-router-dom'; +import { serverType } from '../../servers/prop-types'; +import { shortUrlType } from '../reducers/shortUrlsList'; + +const propTypes = { + shortUrl: shortUrlType, + selectedServer: serverType, + children: PropTypes.node.isRequired, +}; + +const buildVisitsUrl = ({ id }, { shortCode, domain }) => { + const query = domain ? `?domain=${domain}` : ''; + + return `/server/${id}/short-code/${shortCode}/visits${query}`; +}; + +const VisitStatsLink = ({ selectedServer, shortUrl, children, ...rest }) => { + if (!selectedServer || !shortUrl) { + return {children}; + } + + return {children}; +}; + +VisitStatsLink.propTypes = propTypes; + +export default VisitStatsLink; diff --git a/src/short-urls/reducers/shortUrlsList.js b/src/short-urls/reducers/shortUrlsList.js index 6eadeca2..f43c264b 100644 --- a/src/short-urls/reducers/shortUrlsList.js +++ b/src/short-urls/reducers/shortUrlsList.js @@ -18,6 +18,7 @@ export const shortUrlType = PropTypes.shape({ visitsCount: PropTypes.number, meta: shortUrlMetaType, tags: PropTypes.arrayOf(PropTypes.string), + domain: PropTypes.string, }); const initialState = { diff --git a/src/utils/services/ShlinkApiClient.js b/src/utils/services/ShlinkApiClient.js index fe9ce69a..3fdfddfc 100644 --- a/src/utils/services/ShlinkApiClient.js +++ b/src/utils/services/ShlinkApiClient.js @@ -12,6 +12,7 @@ export const apiErrorType = PropTypes.shape({ }); const buildShlinkBaseUrl = (url, apiVersion) => url ? `${url}/rest/v${apiVersion}` : ''; +const rejectNilProps = (options = {}) => reject(isNil, options); export default class ShlinkApiClient { constructor(axios, baseUrl, apiKey) { @@ -22,7 +23,7 @@ export default class ShlinkApiClient { } listShortUrls = pipe( - (options = {}) => reject(isNil, options), + rejectNilProps, (options) => this._performRequest('/short-urls', 'GET', options).then((resp) => resp.data.shortUrls) ); @@ -37,20 +38,20 @@ export default class ShlinkApiClient { this._performRequest(`/short-urls/${shortCode}/visits`, 'GET', query) .then((resp) => resp.data.visits); - getShortUrl = (shortCode) => - this._performRequest(`/short-urls/${shortCode}`, 'GET') + getShortUrl = (shortCode, domain) => + this._performRequest(`/short-urls/${shortCode}`, 'GET', { domain }) .then((resp) => resp.data); - deleteShortUrl = (shortCode) => - this._performRequest(`/short-urls/${shortCode}`, 'DELETE') + deleteShortUrl = (shortCode, domain) => + this._performRequest(`/short-urls/${shortCode}`, 'DELETE', { domain }) .then(() => ({})); - updateShortUrlTags = (shortCode, tags) => - this._performRequest(`/short-urls/${shortCode}/tags`, 'PUT', {}, { tags }) + updateShortUrlTags = (shortCode, domain, tags) => + this._performRequest(`/short-urls/${shortCode}/tags`, 'PUT', { domain }, { tags }) .then((resp) => resp.data.tags); - updateShortUrlMeta = (shortCode, meta) => - this._performRequest(`/short-urls/${shortCode}`, 'PATCH', {}, meta) + updateShortUrlMeta = (shortCode, domain, meta) => + this._performRequest(`/short-urls/${shortCode}`, 'PATCH', { domain }, meta) .then(() => meta); listTags = () => @@ -73,7 +74,7 @@ export default class ShlinkApiClient { method, url: `${buildShlinkBaseUrl(this._baseUrl, this._apiVersion)}${url}`, headers: { 'X-Api-Key': this._apiKey }, - params: query, + params: rejectNilProps(query), data: body, paramsSerializer: (params) => qs.stringify(params, { arrayFormat: 'brackets' }), }); diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index d78fd876..36565e75 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -4,6 +4,7 @@ import { isEmpty, mapObjIndexed, values } from 'ramda'; import React from 'react'; import { Card } from 'reactstrap'; import PropTypes from 'prop-types'; +import qs from 'qs'; import DateRangeRow from '../utils/DateRangeRow'; import MutedMessage from '../utils/MuttedMessage'; import { formatDate } from '../utils/utils'; @@ -21,6 +22,9 @@ const ShortUrlVisits = ( match: PropTypes.shape({ params: PropTypes.object, }), + location: PropTypes.shape({ + search: PropTypes.string, + }), getShortUrlVisits: PropTypes.func, shortUrlVisits: shortUrlVisitsType, getShortUrlDetail: PropTypes.func, @@ -30,14 +34,16 @@ const ShortUrlVisits = ( state = { startDate: undefined, endDate: undefined }; loadVisits = () => { - const { match: { params }, getShortUrlVisits } = this.props; + const { match: { params }, location: { search }, getShortUrlVisits } = this.props; const { shortCode } = params; const dates = mapObjIndexed(formatDate(), this.state); const { startDate, endDate } = dates; + const queryParams = qs.parse(search, { ignoreQueryPrefix: true }); + const { domain } = queryParams; - // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calcs + // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calculations this.memoizationId = `${this.timeWhenMounted}_${shortCode}_${startDate}_${endDate}`; - getShortUrlVisits(shortCode, dates); + getShortUrlVisits(shortCode, { startDate, endDate, domain }); }; componentDidMount() { diff --git a/src/visits/VisitsHeader.js b/src/visits/VisitsHeader.js index 439f2063..e13398dc 100644 --- a/src/visits/VisitsHeader.js +++ b/src/visits/VisitsHeader.js @@ -33,7 +33,7 @@ export default function VisitsHeader({ shortUrlDetail, shortUrlVisits }) {

Visits:{' '} - + Visit stats for

diff --git a/src/visits/reducers/shortUrlVisits.js b/src/visits/reducers/shortUrlVisits.js index faefcc5b..8b23125f 100644 --- a/src/visits/reducers/shortUrlVisits.js +++ b/src/visits/reducers/shortUrlVisits.js @@ -49,7 +49,7 @@ export default handleActions({ [GET_SHORT_URL_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), }, initialState); -export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => async (dispatch, getState) => { +export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, query) => async (dispatch, getState) => { dispatch({ type: GET_SHORT_URL_VISITS_START }); const { getShortUrlVisits } = await buildShlinkApiClient(getState); @@ -57,7 +57,7 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => const isLastPage = ({ currentPage, pagesCount }) => currentPage >= pagesCount; const loadVisits = async (page = 1) => { - const { pagination, data } = await getShortUrlVisits(shortCode, { ...dates, page, itemsPerPage }); + const { pagination, data } = await getShortUrlVisits(shortCode, { ...query, page, itemsPerPage }); // If pagination was not returned, then this is an older shlink version. Just return data if (!pagination || isLastPage(pagination)) { @@ -96,7 +96,7 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => const loadVisitsInParallel = (pages) => Promise.all(pages.map( (page) => - getShortUrlVisits(shortCode, { ...dates, page, itemsPerPage }) + getShortUrlVisits(shortCode, { ...query, page, itemsPerPage }) .then(prop('data')) )).then(flatten); diff --git a/test/short-urls/helpers/ShortUrlVisitsCount.test.js b/test/short-urls/helpers/ShortUrlVisitsCount.test.js index 42508219..3a375fd9 100644 --- a/test/short-urls/helpers/ShortUrlVisitsCount.test.js +++ b/test/short-urls/helpers/ShortUrlVisitsCount.test.js @@ -7,8 +7,8 @@ import ShortUrlVisitsCount from '../../../src/short-urls/helpers/ShortUrlVisitsC describe('', () => { let wrapper; - const createWrapper = (visitsCount, meta) => { - wrapper = shallow(); + const createWrapper = (visitsCount, shortUrl) => { + wrapper = shallow(); return wrapper; }; @@ -17,11 +17,11 @@ describe('', () => { each([ undefined, {}]).it('just returns visits when no maxVisits is provided', (meta) => { const visitsCount = 45; - const wrapper = createWrapper(visitsCount, meta); + const wrapper = createWrapper(visitsCount, { meta }); const maxVisitsHelper = wrapper.find('.short-urls-visits-count__max-visits-control'); const maxVisitsTooltip = wrapper.find(UncontrolledTooltip); - expect(wrapper.html()).toEqual(`${visitsCount}`); + expect(wrapper.html()).toEqual(`${visitsCount}`); expect(maxVisitsHelper).toHaveLength(0); expect(maxVisitsTooltip).toHaveLength(0); }); @@ -30,7 +30,7 @@ describe('', () => { const visitsCount = 45; const maxVisits = 500; const meta = { maxVisits }; - const wrapper = createWrapper(visitsCount, meta); + const wrapper = createWrapper(visitsCount, { meta }); const maxVisitsHelper = wrapper.find('.short-urls-visits-count__max-visits-control'); const maxVisitsTooltip = wrapper.find(UncontrolledTooltip); diff --git a/test/short-urls/helpers/ShortUrlsRowMenu.test.js b/test/short-urls/helpers/ShortUrlsRowMenu.test.js index cfcee90d..59d5f91e 100644 --- a/test/short-urls/helpers/ShortUrlsRowMenu.test.js +++ b/test/short-urls/helpers/ShortUrlsRowMenu.test.js @@ -83,4 +83,6 @@ describe('', () => { done(); }); }); + + it('generates expected visits page link', () => {}) }); diff --git a/test/utils/services/ShlinkApiClient.test.js b/test/utils/services/ShlinkApiClient.test.js index 09697bd6..8cda6271 100644 --- a/test/utils/services/ShlinkApiClient.test.js +++ b/test/utils/services/ShlinkApiClient.test.js @@ -1,8 +1,14 @@ +import each from 'jest-each'; import ShlinkApiClient from '../../../src/utils/services/ShlinkApiClient'; describe('ShlinkApiClient', () => { const createAxiosMock = (data) => () => Promise.resolve(data); const createApiClient = (data) => new ShlinkApiClient(createAxiosMock(data)); + const shortCodesWithDomainCombinations = [ + [ 'abc123', null ], + [ 'abc123', undefined ], + [ 'abc123', 'example.com' ], + ]; describe('listShortUrls', () => { it('properly returns short URLs list', async () => { @@ -67,43 +73,45 @@ describe('ShlinkApiClient', () => { }); describe('getShortUrl', () => { - it('properly returns short URL', async () => { + each(shortCodesWithDomainCombinations).it('properly returns short URL', async (shortCode, domain) => { const expectedShortUrl = { foo: 'bar' }; const axiosSpy = jest.fn(createAxiosMock({ data: expectedShortUrl, })); const { getShortUrl } = new ShlinkApiClient(axiosSpy); - const result = await getShortUrl('abc123'); + const result = await getShortUrl(shortCode, domain); expect(expectedShortUrl).toEqual(result); expect(axiosSpy).toHaveBeenCalledWith(expect.objectContaining({ - url: '/short-urls/abc123', + url: `/short-urls/${shortCode}`, method: 'GET', + params: domain ? { domain } : {}, })); }); }); describe('updateShortUrlTags', () => { - it('properly updates short URL tags', async () => { + each(shortCodesWithDomainCombinations).it('properly updates short URL tags', async (shortCode, domain) => { const expectedTags = [ 'foo', 'bar' ]; const axiosSpy = jest.fn(createAxiosMock({ data: { tags: expectedTags }, })); const { updateShortUrlTags } = new ShlinkApiClient(axiosSpy); - const result = await updateShortUrlTags('abc123', expectedTags); + const result = await updateShortUrlTags(shortCode, domain, expectedTags); expect(expectedTags).toEqual(result); expect(axiosSpy).toHaveBeenCalledWith(expect.objectContaining({ - url: '/short-urls/abc123/tags', + url: `/short-urls/${shortCode}/tags`, method: 'PUT', + params: domain ? { domain } : {}, })); }); }); describe('updateShortUrlMeta', () => { - it('properly updates short URL meta', async () => { + each(shortCodesWithDomainCombinations).it('properly updates short URL meta', async (shortCode, domain) => { const expectedMeta = { maxVisits: 50, validSince: '2025-01-01T10:00:00+01:00', @@ -111,12 +119,13 @@ describe('ShlinkApiClient', () => { const axiosSpy = jest.fn(createAxiosMock()); const { updateShortUrlMeta } = new ShlinkApiClient(axiosSpy); - const result = await updateShortUrlMeta('abc123', expectedMeta); + const result = await updateShortUrlMeta(shortCode, domain, expectedMeta); expect(expectedMeta).toEqual(result); expect(axiosSpy).toHaveBeenCalledWith(expect.objectContaining({ - url: '/short-urls/abc123', + url: `/short-urls/${shortCode}`, method: 'PATCH', + params: domain ? { domain } : {}, })); }); }); @@ -172,15 +181,16 @@ describe('ShlinkApiClient', () => { }); describe('deleteShortUrl', () => { - it('properly deletes provided short URL', async () => { + each(shortCodesWithDomainCombinations).it('properly deletes provided short URL', async (shortCode, domain) => { const axiosSpy = jest.fn(createAxiosMock({})); const { deleteShortUrl } = new ShlinkApiClient(axiosSpy); - await deleteShortUrl('abc123'); + await deleteShortUrl(shortCode, domain); expect(axiosSpy).toHaveBeenCalledWith(expect.objectContaining({ - url: '/short-urls/abc123', + url: `/short-urls/${shortCode}`, method: 'DELETE', + params: domain ? { domain } : {}, })); }); }); diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index 40d634c5..b136efde 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -17,15 +17,17 @@ describe('', () => { const match = { params: { shortCode: 'abc123' }, }; + const location = { search: '' }; const createComponent = (shortUrlVisits) => { - const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits }); + const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits }, () => ''); wrapper = shallow( ', () => { it('shows the amount of visits', () => { const visitsBadge = wrapper.find('.badge'); - expect(visitsBadge.html()).toContain(`Visits: ${shortUrlVisits.visits.length}`); + expect(visitsBadge.html()).toContain(`Visits: ${shortUrlVisits.visits.length}`); }); it('shows when the URL was created', () => { From 707c9f4ce64974bb7c4cd289f6d4fb04d34e0007 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:20:19 +0100 Subject: [PATCH 2/9] Created VisitStatsLink test --- .../helpers/ShortUrlsRowMenu.test.js | 2 - .../short-urls/helpers/VisitStatsLink.test.js | 42 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 test/short-urls/helpers/VisitStatsLink.test.js diff --git a/test/short-urls/helpers/ShortUrlsRowMenu.test.js b/test/short-urls/helpers/ShortUrlsRowMenu.test.js index 59d5f91e..cfcee90d 100644 --- a/test/short-urls/helpers/ShortUrlsRowMenu.test.js +++ b/test/short-urls/helpers/ShortUrlsRowMenu.test.js @@ -83,6 +83,4 @@ describe('', () => { done(); }); }); - - it('generates expected visits page link', () => {}) }); diff --git a/test/short-urls/helpers/VisitStatsLink.test.js b/test/short-urls/helpers/VisitStatsLink.test.js new file mode 100644 index 00000000..dd7a2ec6 --- /dev/null +++ b/test/short-urls/helpers/VisitStatsLink.test.js @@ -0,0 +1,42 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import each from 'jest-each'; +import { Link } from 'react-router-dom'; +import VisitStatsLink from '../../../src/short-urls/helpers/VisitStatsLink'; + +describe('', () => { + let wrapper; + + afterEach(() => wrapper && wrapper.unmount()); + + each([ + [ undefined, undefined ], + [ null, null ], + [{}, null ], + [{}, undefined ], + [ null, {}], + [ undefined, {}], + ]).it('only renders a plan span when either server or short URL are not set', (selectedServer, shortUrl) => { + wrapper = shallow(Something); + const link = wrapper.find(Link); + + expect(link).toHaveLength(0); + expect(wrapper.html()).toEqual('Something'); + }); + + each([ + [{ id: '1' }, { shortCode: 'abc123' }, '/server/1/short-code/abc123/visits' ], + [ + { id: '3' }, + { shortCode: 'def456', domain: 'example.com' }, + '/server/3/short-code/def456/visits?domain=example.com', + ], + ]).it('renders link with expected query when', (selectedServer, shortUrl, expectedLink) => { + wrapper = shallow(Something); + const link = wrapper.find(Link); + const to = link.prop('to'); + + expect(link).toHaveLength(1); + expect(to).toEqual(expectedLink); + }); +}); From 170e427530ee0d5211ca3889798bda1889d4d460 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:38:19 +0100 Subject: [PATCH 3/9] Ensured domain is passed when loading detail for a short URL on a specific domain --- src/visits/ShortUrlVisits.js | 14 +++++++------- src/visits/reducers/shortUrlDetail.js | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 36565e75..df8ba732 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -33,8 +33,8 @@ const ShortUrlVisits = ( }; state = { startDate: undefined, endDate: undefined }; - loadVisits = () => { - const { match: { params }, location: { search }, getShortUrlVisits } = this.props; + loadVisits = (loadDetail = false) => { + const { match: { params }, location: { search }, getShortUrlVisits, getShortUrlDetail } = this.props; const { shortCode } = params; const dates = mapObjIndexed(formatDate(), this.state); const { startDate, endDate } = dates; @@ -44,15 +44,15 @@ const ShortUrlVisits = ( // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calculations this.memoizationId = `${this.timeWhenMounted}_${shortCode}_${startDate}_${endDate}`; getShortUrlVisits(shortCode, { startDate, endDate, domain }); + + if (loadDetail) { + getShortUrlDetail(shortCode, domain); + } }; componentDidMount() { - const { match: { params }, getShortUrlDetail } = this.props; - const { shortCode } = params; - this.timeWhenMounted = new Date().getTime(); - this.loadVisits(); - getShortUrlDetail(shortCode); + this.loadVisits(true); } componentWillUnmount() { diff --git a/src/visits/reducers/shortUrlDetail.js b/src/visits/reducers/shortUrlDetail.js index 893c12ac..2f22243c 100644 --- a/src/visits/reducers/shortUrlDetail.js +++ b/src/visits/reducers/shortUrlDetail.js @@ -26,13 +26,13 @@ export default handleActions({ [GET_SHORT_URL_DETAIL]: (state, { shortUrl }) => ({ shortUrl, loading: false, error: false }), }, initialState); -export const getShortUrlDetail = (buildShlinkApiClient) => (shortCode) => async (dispatch, getState) => { +export const getShortUrlDetail = (buildShlinkApiClient) => (shortCode, domain) => async (dispatch, getState) => { dispatch({ type: GET_SHORT_URL_DETAIL_START }); const { getShortUrl } = await buildShlinkApiClient(getState); try { - const shortUrl = await getShortUrl(shortCode); + const shortUrl = await getShortUrl(shortCode, domain); dispatch({ shortUrl, type: GET_SHORT_URL_DETAIL }); } catch (e) { From 3b95e8ebc04250a5b90a2315f50e3996c7545d19 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:48:35 +0100 Subject: [PATCH 4/9] Ensured domain is passed when editing tags for a short URL on a specific domain --- src/short-urls/helpers/EditTagsModal.js | 2 +- src/short-urls/reducers/shortUrlMeta.js | 2 +- src/short-urls/reducers/shortUrlTags.js | 4 ++-- test/short-urls/helpers/EditTagsModal.test.js | 10 ++++++---- test/short-urls/reducers/shortUrlMeta.test.js | 4 ++-- test/short-urls/reducers/shortUrlTags.test.js | 11 ++++++----- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/short-urls/helpers/EditTagsModal.js b/src/short-urls/helpers/EditTagsModal.js index 141b66ff..8dabb1c1 100644 --- a/src/short-urls/helpers/EditTagsModal.js +++ b/src/short-urls/helpers/EditTagsModal.js @@ -19,7 +19,7 @@ const EditTagsModal = (TagsSelector) => class EditTagsModal extends React.Compon saveTags = () => { const { editShortUrlTags, shortUrl, toggle } = this.props; - editShortUrlTags(shortUrl.shortCode, this.state.tags) + editShortUrlTags(shortUrl.shortCode, shortUrl.domain, this.state.tags) .then(toggle) .catch(() => {}); }; diff --git a/src/short-urls/reducers/shortUrlMeta.js b/src/short-urls/reducers/shortUrlMeta.js index 2c799e13..1fc962b9 100644 --- a/src/short-urls/reducers/shortUrlMeta.js +++ b/src/short-urls/reducers/shortUrlMeta.js @@ -40,7 +40,7 @@ export const editShortUrlMeta = (buildShlinkApiClient) => (shortCode, meta) => a const { updateShortUrlMeta } = await buildShlinkApiClient(getState); try { - await updateShortUrlMeta(shortCode, meta); + await updateShortUrlMeta(shortCode, undefined, meta); dispatch({ shortCode, meta, type: SHORT_URL_META_EDITED }); } catch (e) { dispatch({ type: EDIT_SHORT_URL_META_ERROR }); diff --git a/src/short-urls/reducers/shortUrlTags.js b/src/short-urls/reducers/shortUrlTags.js index 8530e954..77302d76 100644 --- a/src/short-urls/reducers/shortUrlTags.js +++ b/src/short-urls/reducers/shortUrlTags.js @@ -29,12 +29,12 @@ export default handleActions({ [RESET_EDIT_SHORT_URL_TAGS]: () => initialState, }, initialState); -export const editShortUrlTags = (buildShlinkApiClient) => (shortCode, tags) => async (dispatch, getState) => { +export const editShortUrlTags = (buildShlinkApiClient) => (shortCode, domain, tags) => async (dispatch, getState) => { dispatch({ type: EDIT_SHORT_URL_TAGS_START }); const { updateShortUrlTags } = await buildShlinkApiClient(getState); try { - const normalizedTags = await updateShortUrlTags(shortCode, tags); + const normalizedTags = await updateShortUrlTags(shortCode, domain, tags); dispatch({ tags: normalizedTags, shortCode, type: SHORT_URL_TAGS_EDITED }); } catch (e) { diff --git a/test/short-urls/helpers/EditTagsModal.test.js b/test/short-urls/helpers/EditTagsModal.test.js index f6e96a5e..4fd3f499 100644 --- a/test/short-urls/helpers/EditTagsModal.test.js +++ b/test/short-urls/helpers/EditTagsModal.test.js @@ -2,6 +2,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { Modal } from 'reactstrap'; import createEditTagsModal from '../../../src/short-urls/helpers/EditTagsModal'; +import each from 'jest-each'; describe('', () => { let wrapper; @@ -10,7 +11,7 @@ describe('', () => { const editShortUrlTags = jest.fn(() => Promise.resolve()); const resetShortUrlsTags = jest.fn(); const toggle = jest.fn(); - const createWrapper = (shortUrlTags) => { + const createWrapper = (shortUrlTags, domain) => { const EditTagsModal = createEditTagsModal(TagsSelector); wrapper = shallow( @@ -19,6 +20,7 @@ describe('', () => { shortUrl={{ tags: [], shortCode, + domain, originalUrl: 'https://long-domain.com/foo/bar', }} shortUrlTags={shortUrlTags} @@ -74,19 +76,19 @@ describe('', () => { expect(saveBtn.text()).toEqual('Saving tags...'); }); - it('saves tags when save button is clicked', (done) => { + each([[ undefined ], [ null ], [ 'example.com' ]]).it('saves tags when save button is clicked', (domain, done) => { const wrapper = createWrapper({ shortCode, tags: [], saving: true, error: false, - }); + }, domain); const saveBtn = wrapper.find('.btn-primary'); saveBtn.simulate('click'); expect(editShortUrlTags).toHaveBeenCalledTimes(1); - expect(editShortUrlTags).toHaveBeenCalledWith(shortCode, []); + expect(editShortUrlTags).toHaveBeenCalledWith(shortCode, domain, []); // Wrap this expect in a setImmediate since it is called as a result of an inner promise setImmediate(() => { diff --git a/test/short-urls/reducers/shortUrlMeta.test.js b/test/short-urls/reducers/shortUrlMeta.test.js index d1158efd..508bb836 100644 --- a/test/short-urls/reducers/shortUrlMeta.test.js +++ b/test/short-urls/reducers/shortUrlMeta.test.js @@ -61,7 +61,7 @@ describe('shortUrlMetaReducer', () => { expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrlMeta).toHaveBeenCalledTimes(1); - expect(updateShortUrlMeta).toHaveBeenCalledWith(shortCode, meta); + expect(updateShortUrlMeta).toHaveBeenCalledWith(shortCode, undefined, meta); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_META_START }); expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_META_EDITED, meta, shortCode }); @@ -80,7 +80,7 @@ describe('shortUrlMetaReducer', () => { expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrlMeta).toHaveBeenCalledTimes(1); - expect(updateShortUrlMeta).toHaveBeenCalledWith(shortCode, meta); + expect(updateShortUrlMeta).toHaveBeenCalledWith(shortCode, undefined, meta); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_META_START }); expect(dispatch).toHaveBeenNthCalledWith(2, { type: EDIT_SHORT_URL_META_ERROR }); diff --git a/test/short-urls/reducers/shortUrlTags.test.js b/test/short-urls/reducers/shortUrlTags.test.js index 1a80e583..bd43199a 100644 --- a/test/short-urls/reducers/shortUrlTags.test.js +++ b/test/short-urls/reducers/shortUrlTags.test.js @@ -1,3 +1,4 @@ +import each from 'jest-each'; import reducer, { EDIT_SHORT_URL_TAGS_ERROR, EDIT_SHORT_URL_TAGS_START, @@ -60,16 +61,16 @@ describe('shortUrlTagsReducer', () => { dispatch.mockReset(); }); - it('dispatches normalized tags on success', async () => { + each([[ undefined ], [ null ], [ 'example.com' ]]).it('dispatches normalized tags on success', async (domain) => { const normalizedTags = [ 'bar', 'foo' ]; updateShortUrlTags.mockResolvedValue(normalizedTags); - await editShortUrlTags(buildShlinkApiClient)(shortCode, tags)(dispatch); + await editShortUrlTags(buildShlinkApiClient)(shortCode, domain, tags)(dispatch); expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrlTags).toHaveBeenCalledTimes(1); - expect(updateShortUrlTags).toHaveBeenCalledWith(shortCode, tags); + expect(updateShortUrlTags).toHaveBeenCalledWith(shortCode, domain, tags); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_TAGS_START }); expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_TAGS_EDITED, tags: normalizedTags, shortCode }); @@ -81,14 +82,14 @@ describe('shortUrlTagsReducer', () => { updateShortUrlTags.mockRejectedValue(error); try { - await editShortUrlTags(buildShlinkApiClient)(shortCode, tags)(dispatch); + await editShortUrlTags(buildShlinkApiClient)(shortCode, undefined, tags)(dispatch); } catch (e) { expect(e).toBe(error); } expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrlTags).toHaveBeenCalledTimes(1); - expect(updateShortUrlTags).toHaveBeenCalledWith(shortCode, tags); + expect(updateShortUrlTags).toHaveBeenCalledWith(shortCode, undefined, tags); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_TAGS_START }); expect(dispatch).toHaveBeenNthCalledWith(2, { type: EDIT_SHORT_URL_TAGS_ERROR }); From 861a3c068f09c82bba3258c9cbf1ccc7be063595 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:52:30 +0100 Subject: [PATCH 5/9] Ensured domain is passed when editing meta for a short URL on a specific domain --- src/short-urls/helpers/EditMetaModal.js | 2 +- src/short-urls/reducers/shortUrlMeta.js | 4 ++-- test/short-urls/reducers/shortUrlMeta.test.js | 9 +++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/short-urls/helpers/EditMetaModal.js b/src/short-urls/helpers/EditMetaModal.js index 3d403be8..d1bcb020 100644 --- a/src/short-urls/helpers/EditMetaModal.js +++ b/src/short-urls/helpers/EditMetaModal.js @@ -36,7 +36,7 @@ const EditMetaModal = ( const [ maxVisits, setMaxVisits ] = useState(shortUrl && shortUrl.meta && shortUrl.meta.maxVisits); const close = pipe(resetShortUrlMeta, toggle); - const doEdit = () => editShortUrlMeta(shortUrl.shortCode, { + const doEdit = () => editShortUrlMeta(shortUrl.shortCode, shortUrl.domain, { maxVisits: maxVisits && !isEmpty(maxVisits) ? parseInt(maxVisits) : null, validSince: validSince && formatIsoDate(validSince), validUntil: validUntil && formatIsoDate(validUntil), diff --git a/src/short-urls/reducers/shortUrlMeta.js b/src/short-urls/reducers/shortUrlMeta.js index 1fc962b9..c7891cb8 100644 --- a/src/short-urls/reducers/shortUrlMeta.js +++ b/src/short-urls/reducers/shortUrlMeta.js @@ -35,12 +35,12 @@ export default handleActions({ [RESET_EDIT_SHORT_URL_META]: () => initialState, }, initialState); -export const editShortUrlMeta = (buildShlinkApiClient) => (shortCode, meta) => async (dispatch, getState) => { +export const editShortUrlMeta = (buildShlinkApiClient) => (shortCode, domain, meta) => async (dispatch, getState) => { dispatch({ type: EDIT_SHORT_URL_META_START }); const { updateShortUrlMeta } = await buildShlinkApiClient(getState); try { - await updateShortUrlMeta(shortCode, undefined, meta); + await updateShortUrlMeta(shortCode, domain, meta); dispatch({ shortCode, meta, type: SHORT_URL_META_EDITED }); } catch (e) { dispatch({ type: EDIT_SHORT_URL_META_ERROR }); diff --git a/test/short-urls/reducers/shortUrlMeta.test.js b/test/short-urls/reducers/shortUrlMeta.test.js index 508bb836..d3019395 100644 --- a/test/short-urls/reducers/shortUrlMeta.test.js +++ b/test/short-urls/reducers/shortUrlMeta.test.js @@ -1,4 +1,5 @@ import moment from 'moment'; +import each from 'jest-each'; import reducer, { EDIT_SHORT_URL_META_START, EDIT_SHORT_URL_META_ERROR, @@ -56,12 +57,12 @@ describe('shortUrlMetaReducer', () => { afterEach(jest.clearAllMocks); - it('dispatches metadata on success', async () => { - await editShortUrlMeta(buildShlinkApiClient)(shortCode, meta)(dispatch); + each([[ undefined ], [ null ], [ 'example.com' ]]).it('dispatches metadata on success', async (domain) => { + await editShortUrlMeta(buildShlinkApiClient)(shortCode, domain, meta)(dispatch); expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrlMeta).toHaveBeenCalledTimes(1); - expect(updateShortUrlMeta).toHaveBeenCalledWith(shortCode, undefined, meta); + expect(updateShortUrlMeta).toHaveBeenCalledWith(shortCode, domain, meta); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_META_START }); expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_META_EDITED, meta, shortCode }); @@ -73,7 +74,7 @@ describe('shortUrlMetaReducer', () => { updateShortUrlMeta.mockRejectedValue(error); try { - await editShortUrlMeta(buildShlinkApiClient)(shortCode, meta)(dispatch); + await editShortUrlMeta(buildShlinkApiClient)(shortCode, undefined, meta)(dispatch); } catch (e) { expect(e).toBe(error); } From 098c94bccf33398023ac0dae77cb6b7df8575ad3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:57:18 +0100 Subject: [PATCH 6/9] Ensured domain is passed when deleting a short URL on a specific domain --- src/short-urls/helpers/DeleteShortUrlModal.js | 4 ++-- src/short-urls/reducers/shortUrlDeletion.js | 4 ++-- test/short-urls/reducers/shortUrlDeletion.test.js | 11 +++++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/short-urls/helpers/DeleteShortUrlModal.js b/src/short-urls/helpers/DeleteShortUrlModal.js index a7a0affb..6121b9a2 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.js +++ b/src/short-urls/helpers/DeleteShortUrlModal.js @@ -22,9 +22,9 @@ export default class DeleteShortUrlModal extends React.Component { e.preventDefault(); const { deleteShortUrl, shortUrl, toggle } = this.props; - const { shortCode } = shortUrl; + const { shortCode, domain } = shortUrl; - deleteShortUrl(shortCode) + deleteShortUrl(shortCode, domain) .then(toggle) .catch(identity); }; diff --git a/src/short-urls/reducers/shortUrlDeletion.js b/src/short-urls/reducers/shortUrlDeletion.js index 9d86ccf1..6754527a 100644 --- a/src/short-urls/reducers/shortUrlDeletion.js +++ b/src/short-urls/reducers/shortUrlDeletion.js @@ -30,13 +30,13 @@ export default handleActions({ [RESET_DELETE_SHORT_URL]: () => initialState, }, initialState); -export const deleteShortUrl = (buildShlinkApiClient) => (shortCode) => async (dispatch, getState) => { +export const deleteShortUrl = (buildShlinkApiClient) => (shortCode, domain) => async (dispatch, getState) => { dispatch({ type: DELETE_SHORT_URL_START }); const { deleteShortUrl } = await buildShlinkApiClient(getState); try { - await deleteShortUrl(shortCode); + await deleteShortUrl(shortCode, domain); dispatch({ type: SHORT_URL_DELETED, shortCode }); } catch (e) { dispatch({ type: DELETE_SHORT_URL_ERROR, errorData: e.response.data }); diff --git a/test/short-urls/reducers/shortUrlDeletion.test.js b/test/short-urls/reducers/shortUrlDeletion.test.js index 4c62133b..be7f3adf 100644 --- a/test/short-urls/reducers/shortUrlDeletion.test.js +++ b/test/short-urls/reducers/shortUrlDeletion.test.js @@ -1,3 +1,4 @@ +import each from 'jest-each'; import reducer, { DELETE_SHORT_URL_ERROR, DELETE_SHORT_URL_START, @@ -59,20 +60,22 @@ describe('shortUrlDeletionReducer', () => { getState.mockClear(); }); - it('dispatches proper actions if API client request succeeds', async () => { + each( + [[ undefined ], [ null ], [ 'example.com' ]] + ).it('dispatches proper actions if API client request succeeds', async (domain) => { const apiClientMock = { deleteShortUrl: jest.fn(() => ''), }; const shortCode = 'abc123'; - await deleteShortUrl(() => apiClientMock)(shortCode)(dispatch, getState); + await deleteShortUrl(() => apiClientMock)(shortCode, domain)(dispatch, getState); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: DELETE_SHORT_URL_START }); expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_DELETED, shortCode }); expect(apiClientMock.deleteShortUrl).toHaveBeenCalledTimes(1); - expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode); + expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode, domain); }); it('dispatches proper actions if API client request fails', async () => { @@ -94,7 +97,7 @@ describe('shortUrlDeletionReducer', () => { expect(dispatch).toHaveBeenNthCalledWith(2, { type: DELETE_SHORT_URL_ERROR, errorData: data }); expect(apiClientMock.deleteShortUrl).toHaveBeenCalledTimes(1); - expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode); + expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode, undefined); }); }); }); From 58077f2d861a7de47a2ce24ab05dbc406b5870b2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 09:59:13 +0100 Subject: [PATCH 7/9] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 966a248d..21bbbc76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [#193](https://github.com/shlinkio/shlink-web-client/issues/193) Fixed `maxVisits` being set to 0 when trying to reset it from having a value to `null`. * [#196](https://github.com/shlinkio/shlink-web-client/issues/196) Included apache `.htaccess` file which takes care of falling back to index.html when reloading the page on a client-side handled route. +* [#179](https://github.com/shlinkio/shlink-web-client/issues/179) Ensured domain is provided to Shlink server when editing, deleting or fetching short URLs which do not belong to default domain. ## 2.3.0 - 2020-01-19 From c67ce3918b1590a8c99bfc1e82dae36acd12ab4a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 10:02:33 +0100 Subject: [PATCH 8/9] Removed redundant function call --- src/utils/services/ShlinkApiClient.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/utils/services/ShlinkApiClient.js b/src/utils/services/ShlinkApiClient.js index 3fdfddfc..fc9d5652 100644 --- a/src/utils/services/ShlinkApiClient.js +++ b/src/utils/services/ShlinkApiClient.js @@ -1,5 +1,5 @@ import qs from 'qs'; -import { isEmpty, isNil, pipe, reject } from 'ramda'; +import { isEmpty, isNil, reject } from 'ramda'; import PropTypes from 'prop-types'; export const apiErrorType = PropTypes.shape({ @@ -12,7 +12,7 @@ export const apiErrorType = PropTypes.shape({ }); const buildShlinkBaseUrl = (url, apiVersion) => url ? `${url}/rest/v${apiVersion}` : ''; -const rejectNilProps = (options = {}) => reject(isNil, options); +const rejectNilProps = reject(isNil); export default class ShlinkApiClient { constructor(axios, baseUrl, apiKey) { @@ -22,10 +22,8 @@ export default class ShlinkApiClient { this._apiKey = apiKey || ''; } - listShortUrls = pipe( - rejectNilProps, - (options) => this._performRequest('/short-urls', 'GET', options).then((resp) => resp.data.shortUrls) - ); + listShortUrls = (options = {}) => + this._performRequest('/short-urls', 'GET', options).then((resp) => resp.data.shortUrls); createShortUrl = (options) => { const filteredOptions = reject((value) => isEmpty(value) || isNil(value), options); From 30e5253acd09fe7008d76fd8eac1cc454c9a1a83 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 8 Feb 2020 10:07:34 +0100 Subject: [PATCH 9/9] Simplified instructions removing redundant vars --- src/visits/ShortUrlVisits.js | 6 ++---- test/short-urls/helpers/EditTagsModal.test.js | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index df8ba732..7b1ae484 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -36,10 +36,8 @@ const ShortUrlVisits = ( loadVisits = (loadDetail = false) => { const { match: { params }, location: { search }, getShortUrlVisits, getShortUrlDetail } = this.props; const { shortCode } = params; - const dates = mapObjIndexed(formatDate(), this.state); - const { startDate, endDate } = dates; - const queryParams = qs.parse(search, { ignoreQueryPrefix: true }); - const { domain } = queryParams; + const { startDate, endDate } = mapObjIndexed(formatDate(), this.state); + const { domain } = qs.parse(search, { ignoreQueryPrefix: true }); // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calculations this.memoizationId = `${this.timeWhenMounted}_${shortCode}_${startDate}_${endDate}`; diff --git a/test/short-urls/helpers/EditTagsModal.test.js b/test/short-urls/helpers/EditTagsModal.test.js index 4fd3f499..07243442 100644 --- a/test/short-urls/helpers/EditTagsModal.test.js +++ b/test/short-urls/helpers/EditTagsModal.test.js @@ -1,8 +1,8 @@ import React from 'react'; import { shallow } from 'enzyme'; import { Modal } from 'reactstrap'; -import createEditTagsModal from '../../../src/short-urls/helpers/EditTagsModal'; import each from 'jest-each'; +import createEditTagsModal from '../../../src/short-urls/helpers/EditTagsModal'; describe('', () => { let wrapper;