Merge pull request #168 from acelaya-forks/feature/default-servers-issue

Feature/default servers issue

.github/ISSUE_TEMPLATE/Bug.md

@@ -16,6 +16,7 @@ With that said, please fill in the information requested next. More information
 #### Shlink web client version
 
 * Version: x.y.z
+* How do you use shlink-web-client: app.shlink.io|Docker image|self-hosted
 
 #### Summary
 

.github/ISSUE_TEMPLATE/Question_Support.md

@@ -16,6 +16,7 @@ With that said, please fill in the information requested next. More information
 #### Shlink web client version
 
 * Version: x.y.z
+* How do you use shlink-web-client: app.shlink.io|Docker image|self-hosted
 
 #### Summary
 

CHANGELOG.md

@@ -4,6 +4,52 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## 2.2.2 - 2019-10-21
+
+#### Added
+
+* *Nothing*
+
+#### Changed
+
+* *Nothing*
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* [#167](https://github.com/shlinkio/shlink-web-client/issues/167) Fixed `/servers.json` path not being ignored when returning something other than an array.
+
+
+## 2.2.1 - 2019-10-18
+
+#### Added
+
+* *Nothing*
+
+#### Changed
+
+* *Nothing*
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* [#165](https://github.com/shlinkio/shlink-web-client/issues/165) Fixed error thrown when opening "create" page while using a Shlink version which does not return a valid SemVer version (like `latest` docker image, or any development instance).
+
+
 ## 2.2.0 - 2019-10-05
 
 #### Added

config/docker/nginx.conf

@@ -5,7 +5,7 @@ server {
     index index.html;
 
     # When requesting static paths with extension, try them, and return a 404 if not found
-    location ~ .+\.(css|js|html|png|jpg|jpeg|gif|bmp|ico|csv|otf|eot|svg|svgz|ttf|woff|woff2|ijmap|pdf|tif|map) {
+    location ~ .+\.(css|js|html|png|jpg|jpeg|gif|bmp|ico|json|csv|otf|eot|svg|svgz|ttf|woff|woff2|ijmap|pdf|tif|map) {
         try_files $uri $uri/ =404;
     }
 

src/servers/reducers/selectedServer.js

@@ -1,9 +1,14 @@
 import { createAction, handleActions } from 'redux-actions';
 import { resetShortUrlParams } from '../../short-urls/reducers/shortUrlsListParams';
+import { versionIsValidSemVer } from '../../utils/utils';
 
 /* eslint-disable padding-line-between-statements */
 export const SELECT_SERVER = 'shlink/selectedServer/SELECT_SERVER';
 export const RESET_SELECTED_SERVER = 'shlink/selectedServer/RESET_SELECTED_SERVER';
+
+export const MIN_FALLBACK_VERSION = '1.0.0';
+export const MAX_FALLBACK_VERSION = '999.999.999';
+export const LATEST_VERSION_CONSTRAINT = 'latest';
 /* eslint-enable padding-line-between-statements */
 
 const initialState = null;
@@ -15,7 +20,10 @@ export const selectServer = ({ findServerById }, buildShlinkApiClient) => (serve
 
   const selectedServer = findServerById(serverId);
   const { health } = await buildShlinkApiClient(selectedServer);
-  const { version } = await health().catch(() => ({ version: '1.0.0' }));
+  const version = await health()
+    .then(({ version }) => version === LATEST_VERSION_CONSTRAINT ? MAX_FALLBACK_VERSION : version)
+    .then((version) => !versionIsValidSemVer(version) ? MIN_FALLBACK_VERSION : version)
+    .catch(() => MIN_FALLBACK_VERSION);
 
   dispatch({
     type: SELECT_SERVER,

src/servers/reducers/server.js

@@ -30,10 +30,20 @@ export const listServers = ({ listServers, createServers }, { get }) => () => as
     return;
   }
 
-  // If local list is empty, try to fetch it remotely and calculate IDs for every server
+  // If local list is empty, try to fetch it remotely (making sure it's an array) and calculate IDs for every server
+  const getDataAsArrayWithIds = pipe(
+    prop('data'),
+    (value) => {
+      if (!Array.isArray(value)) {
+        throw new Error('Value is not an array');
+      }
+
+      return value;
+    },
+    map(assocId),
+  );
   const remoteList = await get(`${homepage}/servers.json`)
-    .then(prop('data'))
-    .then(map(assocId))
+    .then(getDataAsArrayWithIds)
     .catch(() => []);
 
   createServers(remoteList);

src/utils/utils.js

@@ -60,3 +60,11 @@ export const compareVersions = (firstVersion, operator, secondVersion) => compar
   secondVersion,
   operator
 );
+
+export const versionIsValidSemVer = (version) => {
+  try {
+    return compareVersions(version, '=', version);
+  } catch (e) {
+    return false;
+  }
+};

test/servers/reducers/selectedServer.test.js

@@ -1,8 +1,11 @@
+import each from 'jest-each';
 import reducer, {
   selectServer,
   resetSelectedServer,
   RESET_SELECTED_SERVER,
   SELECT_SERVER,
+  MAX_FALLBACK_VERSION,
+  MIN_FALLBACK_VERSION,
 } from '../../../src/servers/reducers/selectedServer';
 import { RESET_SHORT_URL_PARAMS } from '../../../src/short-urls/reducers/shortUrlsListParams';
 
@@ -34,26 +37,25 @@ describe('selectedServerReducer', () => {
       findServerById: jest.fn(() => selectedServer),
     };
     const apiClientMock = {
-      health: jest.fn().mockResolvedValue({ version }),
+      health: jest.fn(),
     };
     const buildApiClient = jest.fn().mockResolvedValue(apiClientMock);
+    const dispatch = jest.fn();
 
-    beforeEach(() => {
-      apiClientMock.health.mockClear();
-      buildApiClient.mockClear();
-    });
+    afterEach(jest.clearAllMocks);
 
-    afterEach(() => {
-      ServersServiceMock.findServerById.mockClear();
-    });
-
-    it('dispatches proper actions', async () => {
-      const dispatch = jest.fn();
+    each([
+      [ version, version ],
+      [ 'latest', MAX_FALLBACK_VERSION ],
+      [ '%invalid_semver%', MIN_FALLBACK_VERSION ],
+    ]).it('dispatches proper actions', async (serverVersion, expectedVersion) => {
       const expectedSelectedServer = {
         ...selectedServer,
-        version,
+        version: expectedVersion,
       };
 
+      apiClientMock.health.mockResolvedValue({ version: serverVersion });
+
       await selectServer(ServersServiceMock, buildApiClient)(serverId)(dispatch);
 
       expect(dispatch).toHaveBeenCalledTimes(2);
@@ -67,5 +69,18 @@ describe('selectedServerReducer', () => {
       expect(ServersServiceMock.findServerById).toHaveBeenCalledTimes(1);
       expect(buildApiClient).toHaveBeenCalledTimes(1);
     });
+
+    it('falls back to min version when health endpoint fails', async () => {
+      const expectedSelectedServer = {
+        ...selectedServer,
+        version: MIN_FALLBACK_VERSION,
+      };
+
+      apiClientMock.health.mockRejectedValue({});
+
+      await selectServer(ServersServiceMock, buildApiClient)(serverId)(dispatch);
+
+      expect(dispatch).toHaveBeenNthCalledWith(2, { type: SELECT_SERVER, selectedServer: expectedSelectedServer });
+    });
   });
 });

test/servers/reducers/server.test.js

@@ -1,4 +1,5 @@
 import { values } from 'ramda';
+import each from 'jest-each';
 import reducer, {
   createServer,
   deleteServer,
@@ -20,27 +21,18 @@ describe('serverReducer', () => {
     createServers: jest.fn(),
   };
 
+  afterEach(jest.clearAllMocks);
+
   describe('reducer', () => {
     it('returns servers when action is FETCH_SERVERS', () =>
       expect(reducer({}, { type: FETCH_SERVERS, list })).toEqual({ loading: false, list }));
   });
 
   describe('action creators', () => {
-    beforeEach(() => {
-      ServersServiceMock.listServers.mockClear();
-      ServersServiceMock.createServer.mockReset();
-      ServersServiceMock.deleteServer.mockReset();
-      ServersServiceMock.createServers.mockReset();
-    });
-
     describe('listServers', () => {
-      const axios = { get: jest.fn().mockResolvedValue({ data: [] }) };
+      const axios = { get: jest.fn() };
       const dispatch = jest.fn();
-
-      beforeEach(() => {
-        axios.get.mockClear();
-        dispatch.mockReset();
-      });
+      const NoListServersServiceMock = { ...ServersServiceMock, listServers: jest.fn(() => ({})) };
 
       it('fetches servers from local storage when found', async () => {
         await listServers(ServersServiceMock, axios)()(dispatch);
@@ -55,14 +47,49 @@ describe('serverReducer', () => {
         expect(axios.get).not.toHaveBeenCalled();
       });
 
-      it('tries to fetch servers from remote when not found locally', async () => {
-        const NoListServersServiceMock = { ...ServersServiceMock, listServers: jest.fn(() => ({})) };
+      each([
+        [
+          Promise.resolve({
+            data: [
+              {
+                id: '111',
+                name: 'acel.me from servers.json',
+                url: 'https://acel.me',
+                apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0',
+              },
+              {
+                id: '222',
+                name: 'Local from servers.json',
+                url: 'http://localhost:8000',
+                apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a',
+              },
+            ],
+          }),
+          {
+            111: {
+              id: '111',
+              name: 'acel.me from servers.json',
+              url: 'https://acel.me',
+              apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0',
+            },
+            222: {
+              id: '222',
+              name: 'Local from servers.json',
+              url: 'http://localhost:8000',
+              apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a',
+            },
+          },
+        ],
+        [ Promise.resolve('<html></html>'), {}],
+        [ Promise.reject({}), {}],
+      ]).it('tries to fetch servers from remote when not found locally', async (mockedValue, expectedList) => {
+        axios.get.mockReturnValue(mockedValue);
 
         await listServers(NoListServersServiceMock, axios)()(dispatch);
 
         expect(dispatch).toHaveBeenCalledTimes(2);
         expect(dispatch).toHaveBeenNthCalledWith(1, { type: FETCH_SERVERS_START });
-        expect(dispatch).toHaveBeenNthCalledWith(2, { type: FETCH_SERVERS, list: {} });
+        expect(dispatch).toHaveBeenNthCalledWith(2, { type: FETCH_SERVERS, list: expectedList });
         expect(NoListServersServiceMock.listServers).toHaveBeenCalledTimes(1);
         expect(NoListServersServiceMock.createServer).not.toHaveBeenCalled();
         expect(NoListServersServiceMock.deleteServer).not.toHaveBeenCalled();