diff --git a/CHANGELOG.md b/CHANGELOG.md index 66806cfc..0ca5ced5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed * [#382](https://github.com/shlinkio/shlink-web-client/issues/382) Ensured short URL tags are edited through the `PATCH /short-urls/{shortCode}` endpoint when using Shlink 2.6.0 or higher. +* [#398](https://github.com/shlinkio/shlink-web-client/issues/398) Improved performance when loading short URL details by avoiding API calls if the short URL is already present in local state. ### Deprecated * *Nothing* diff --git a/src/container/types.ts b/src/container/types.ts index d51764c2..3b4fff27 100644 --- a/src/container/types.ts +++ b/src/container/types.ts @@ -11,7 +11,7 @@ import { ShortUrlsList } from '../short-urls/reducers/shortUrlsList'; import { TagDeletion } from '../tags/reducers/tagDelete'; import { TagEdition } from '../tags/reducers/tagEdit'; import { TagsList } from '../tags/reducers/tagsList'; -import { ShortUrlDetail } from '../visits/reducers/shortUrlDetail'; +import { ShortUrlDetail } from '../short-urls/reducers/shortUrlDetail'; import { ShortUrlVisits } from '../visits/reducers/shortUrlVisits'; import { TagVisits } from '../visits/reducers/tagVisits'; import { DomainsList } from '../domains/reducers/domainsList'; diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 4efcf1c6..29311fa7 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -11,7 +11,7 @@ import shortUrlEditionReducer from '../short-urls/reducers/shortUrlEdition'; import shortUrlVisitsReducer from '../visits/reducers/shortUrlVisits'; import tagVisitsReducer from '../visits/reducers/tagVisits'; import orphanVisitsReducer from '../visits/reducers/orphanVisits'; -import shortUrlDetailReducer from '../visits/reducers/shortUrlDetail'; +import shortUrlDetailReducer from '../short-urls/reducers/shortUrlDetail'; import tagsListReducer from '../tags/reducers/tagsList'; import tagDeleteReducer from '../tags/reducers/tagDelete'; import tagEditReducer from '../tags/reducers/tagEdit'; diff --git a/src/visits/reducers/shortUrlDetail.ts b/src/short-urls/reducers/shortUrlDetail.ts similarity index 83% rename from src/visits/reducers/shortUrlDetail.ts rename to src/short-urls/reducers/shortUrlDetail.ts index c8fb5bea..42771a92 100644 --- a/src/visits/reducers/shortUrlDetail.ts +++ b/src/short-urls/reducers/shortUrlDetail.ts @@ -1,9 +1,10 @@ import { Action, Dispatch } from 'redux'; -import { ShortUrl } from '../../short-urls/data'; +import { ShortUrl } from '../data'; import { buildReducer } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import { OptionalString } from '../../utils/utils'; import { GetState } from '../../container/types'; +import { shortUrlMatches } from '../helpers'; /* eslint-disable padding-line-between-statements */ export const GET_SHORT_URL_DETAIL_START = 'shlink/shortUrlDetail/GET_SHORT_URL_DETAIL_START'; @@ -37,10 +38,12 @@ export const getShortUrlDetail = (buildShlinkApiClient: ShlinkApiClientBuilder) domain: OptionalString, ) => async (dispatch: Dispatch, getState: GetState) => { dispatch({ type: GET_SHORT_URL_DETAIL_START }); - const { getShortUrl } = buildShlinkApiClient(getState); try { - const shortUrl = await getShortUrl(shortCode, domain); + const { shortUrlsList } = getState(); + const shortUrl = shortUrlsList?.shortUrls?.data.find( + (shortUrl) => shortUrlMatches(shortUrl, shortCode, domain), + ) ?? await buildShlinkApiClient(getState).getShortUrl(shortCode, domain); dispatch({ shortUrl, type: GET_SHORT_URL_DETAIL }); } catch (e) { diff --git a/src/visits/ShortUrlVisits.tsx b/src/visits/ShortUrlVisits.tsx index 3d71b8cf..509e34ae 100644 --- a/src/visits/ShortUrlVisits.tsx +++ b/src/visits/ShortUrlVisits.tsx @@ -4,9 +4,9 @@ import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; import { ShlinkVisitsParams } from '../api/types'; import { parseQuery } from '../utils/helpers/query'; import { Topics } from '../mercure/helpers/Topics'; +import { ShortUrlDetail } from '../short-urls/reducers/shortUrlDetail'; import { ShortUrlVisits as ShortUrlVisitsState } from './reducers/shortUrlVisits'; import ShortUrlVisitsHeader from './ShortUrlVisitsHeader'; -import { ShortUrlDetail } from './reducers/shortUrlDetail'; import VisitsStats from './VisitsStats'; export interface ShortUrlVisitsProps extends RouteComponentProps<{ shortCode: string }> { diff --git a/src/visits/ShortUrlVisitsHeader.tsx b/src/visits/ShortUrlVisitsHeader.tsx index 38826b8e..c581f3ed 100644 --- a/src/visits/ShortUrlVisitsHeader.tsx +++ b/src/visits/ShortUrlVisitsHeader.tsx @@ -1,7 +1,7 @@ import { UncontrolledTooltip } from 'reactstrap'; import Moment from 'react-moment'; import { ExternalLink } from 'react-external-link'; -import { ShortUrlDetail } from './reducers/shortUrlDetail'; +import { ShortUrlDetail } from '../short-urls/reducers/shortUrlDetail'; import { ShortUrlVisits } from './reducers/shortUrlVisits'; import VisitsHeader from './VisitsHeader'; import './ShortUrlVisitsHeader.scss'; diff --git a/src/visits/services/provideServices.ts b/src/visits/services/provideServices.ts index f550b68f..5e7775f5 100644 --- a/src/visits/services/provideServices.ts +++ b/src/visits/services/provideServices.ts @@ -1,7 +1,7 @@ import Bottle from 'bottlejs'; import ShortUrlVisits from '../ShortUrlVisits'; import { cancelGetShortUrlVisits, getShortUrlVisits } from '../reducers/shortUrlVisits'; -import { getShortUrlDetail } from '../reducers/shortUrlDetail'; +import { getShortUrlDetail } from '../../short-urls/reducers/shortUrlDetail'; import MapModal from '../helpers/MapModal'; import { createNewVisits } from '../reducers/visitCreation'; import TagVisits from '../TagVisits'; diff --git a/test/visits/reducers/shortUrlDetail.test.ts b/test/short-urls/reducers/shortUrlDetail.test.ts similarity index 62% rename from test/visits/reducers/shortUrlDetail.test.ts rename to test/short-urls/reducers/shortUrlDetail.test.ts index 02d5bf39..a83d1a4d 100644 --- a/test/visits/reducers/shortUrlDetail.test.ts +++ b/test/short-urls/reducers/shortUrlDetail.test.ts @@ -5,12 +5,15 @@ import reducer, { GET_SHORT_URL_DETAIL_ERROR, GET_SHORT_URL_DETAIL, ShortUrlDetailAction, -} from '../../../src/visits/reducers/shortUrlDetail'; +} from '../../../src/short-urls/reducers/shortUrlDetail'; import { ShortUrl } from '../../../src/short-urls/data'; import ShlinkApiClient from '../../../src/api/services/ShlinkApiClient'; import { ShlinkState } from '../../../src/container/types'; +import { ShortUrlsList } from '../../../src/short-urls/reducers/shortUrlsList'; describe('shortUrlDetailReducer', () => { + beforeEach(jest.clearAllMocks); + describe('reducer', () => { const action = (type: string) => Mock.of({ type }); @@ -45,14 +48,12 @@ describe('shortUrlDetailReducer', () => { getShortUrl: jest.fn(async () => returned), }); const dispatchMock = jest.fn(); - const getState = () => Mock.of(); - - beforeEach(() => dispatchMock.mockReset()); + const buildGetState = (shortUrlsList?: ShortUrlsList) => () => Mock.of({ shortUrlsList }); it('dispatches start and error when promise is rejected', async () => { const ShlinkApiClient = buildApiClientMock(Promise.reject()); - await getShortUrlDetail(() => ShlinkApiClient)('abc123', '')(dispatchMock, getState); + await getShortUrlDetail(() => ShlinkApiClient)('abc123', '')(dispatchMock, buildGetState()); expect(dispatchMock).toHaveBeenCalledTimes(2); expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); @@ -60,16 +61,50 @@ describe('shortUrlDetailReducer', () => { expect(ShlinkApiClient.getShortUrl).toHaveBeenCalledTimes(1); }); - it('dispatches start and success when promise is resolved', async () => { - const resolvedShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'bar' }); + it.each([ + [ undefined ], + [ Mock.all() ], + [ + Mock.of({ + shortUrls: { data: [] }, + }), + ], + [ + Mock.of({ + shortUrls: { + data: [ Mock.of({ shortCode: 'this_will_not_match' }) ], + }, + }), + ], + ])('performs API call when short URL is not found in local state', async (shortUrlsList?: ShortUrlsList) => { + const resolvedShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'abc123' }); const ShlinkApiClient = buildApiClientMock(Promise.resolve(resolvedShortUrl)); - await getShortUrlDetail(() => ShlinkApiClient)('abc123', '')(dispatchMock, getState); + await getShortUrlDetail(() => ShlinkApiClient)('abc123', '')(dispatchMock, buildGetState(shortUrlsList)); expect(dispatchMock).toHaveBeenCalledTimes(2); expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, shortUrl: resolvedShortUrl }); expect(ShlinkApiClient.getShortUrl).toHaveBeenCalledTimes(1); }); + + it('avoids API calls when short URL is found in local state', async () => { + const foundShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'abc123' }); + const ShlinkApiClient = buildApiClientMock(Promise.resolve(Mock.all())); + + await getShortUrlDetail(() => ShlinkApiClient)(foundShortUrl.shortCode, foundShortUrl.domain)( + dispatchMock, + buildGetState(Mock.of({ + shortUrls: { + data: [ foundShortUrl ], + }, + })), + ); + + expect(dispatchMock).toHaveBeenCalledTimes(2); + expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); + expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, shortUrl: foundShortUrl }); + expect(ShlinkApiClient.getShortUrl).not.toHaveBeenCalled(); + }); }); }); diff --git a/test/visits/ShortUrlVisits.test.tsx b/test/visits/ShortUrlVisits.test.tsx index 225c2711..2a3be38e 100644 --- a/test/visits/ShortUrlVisits.test.tsx +++ b/test/visits/ShortUrlVisits.test.tsx @@ -6,7 +6,7 @@ import { match } from 'react-router'; // eslint-disable-line @typescript-eslint/ import ShortUrlVisits, { ShortUrlVisitsProps } from '../../src/visits/ShortUrlVisits'; import ShortUrlVisitsHeader from '../../src/visits/ShortUrlVisitsHeader'; import { ShortUrlVisits as ShortUrlVisitsState } from '../../src/visits/reducers/shortUrlVisits'; -import { ShortUrlDetail } from '../../src/visits/reducers/shortUrlDetail'; +import { ShortUrlDetail } from '../../src/short-urls/reducers/shortUrlDetail'; import VisitsStats from '../../src/visits/VisitsStats'; import { MercureBoundProps } from '../../src/mercure/helpers/boundToMercureHub'; diff --git a/test/visits/ShortUrlVisitsHeader.test.tsx b/test/visits/ShortUrlVisitsHeader.test.tsx index c2701eba..e6d4c43a 100644 --- a/test/visits/ShortUrlVisitsHeader.test.tsx +++ b/test/visits/ShortUrlVisitsHeader.test.tsx @@ -3,7 +3,7 @@ import Moment from 'react-moment'; import { ExternalLink } from 'react-external-link'; import { Mock } from 'ts-mockery'; import ShortUrlVisitsHeader from '../../src/visits/ShortUrlVisitsHeader'; -import { ShortUrlDetail } from '../../src/visits/reducers/shortUrlDetail'; +import { ShortUrlDetail } from '../../src/short-urls/reducers/shortUrlDetail'; import { ShortUrlVisits } from '../../src/visits/reducers/shortUrlVisits'; describe('', () => {