From c8d682cc98070c1fef7e33949f743bd95b04a523 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 8 Mar 2020 10:00:25 +0100
Subject: [PATCH] Handled loading server in just one place, and added error
 handling for loading servers

---
 src/common/MenuLayout.js                | 19 +++++++++---
 src/servers/ServersDropdown.js          | 14 ++-------
 src/servers/prop-types/index.js         | 10 ++++---
 src/servers/reducers/selectedServer.js  | 39 ++++++++++++++++++-------
 src/servers/services/provideServices.js |  2 +-
 5 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/src/common/MenuLayout.js b/src/common/MenuLayout.js
index 437f9d02..592a6b9c 100644
--- a/src/common/MenuLayout.js
+++ b/src/common/MenuLayout.js
@@ -20,19 +20,30 @@ const propTypes = {
 const MenuLayout = (TagsList, ShortUrls, AsideMenu, CreateShortUrl, ShortUrlVisits, ShlinkVersions) => {
   const MenuLayoutComp = ({ match, location, selectedServer, selectServer }) => {
     const [ showSideBar, setShowSidebar ] = useState(false);
+    const { params: { serverId } } = match;
 
     useEffect(() => {
-      const { params: { serverId } } = match;
-
       selectServer(serverId);
-    }, []);
+    }, [ serverId ]);
     useEffect(() => setShowSidebar(false), [ location ]);
 
     if (!selectedServer) {
       return <MutedMessage loading />;
     }
 
-    const { params: { serverId } } = match;
+    if (selectedServer.serverNotFound) {
+      return <MutedMessage>Could not find a server with id <b>&quot;{serverId}&quot;</b> in this host.</MutedMessage>;
+    }
+
+    if (selectedServer.serverNotReachable) {
+      return (
+        <MutedMessage>
+          Oops! Could not connect to Shlink server with ID <b>&quot;{serverId}&quot;</b>. Make sure you have internet
+          connection, the server is properly configured and it is on-line.
+        </MutedMessage>
+      );
+    }
+
     const burgerClasses = classNames('menu-layout__burger-icon', {
       'menu-layout__burger-icon--active': showSideBar,
     });
diff --git a/src/servers/ServersDropdown.js b/src/servers/ServersDropdown.js
index 20583787..c66cb22c 100644
--- a/src/servers/ServersDropdown.js
+++ b/src/servers/ServersDropdown.js
@@ -8,7 +8,6 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React
   static propTypes = {
     servers: PropTypes.object,
     selectedServer: serverType,
-    selectServer: PropTypes.func,
     listServers: PropTypes.func,
     history: PropTypes.shape({
       push: PropTypes.func,
@@ -16,14 +15,10 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React
   };
 
   renderServers = () => {
-    const { servers: { list, loading }, selectedServer, selectServer } = this.props;
+    const { servers: { list, loading }, selectedServer } = this.props;
     const servers = values(list);
     const { push } = this.props.history;
-    const loadServer = (id) => {
-      selectServer(id)
-        .then(() => push(`/server/${id}/list-short-urls/1`))
-        .catch(() => {});
-    };
+    const loadServer = (id) => push(`/server/${id}/list-short-urls/1`);
 
     if (loading) {
       return <DropdownItem disabled><i>Trying to load servers...</i></DropdownItem>;
@@ -41,10 +36,7 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React
           </DropdownItem>
         ))}
         <DropdownItem divider />
-        <DropdownItem
-          className="servers-dropdown__export-item"
-          onClick={() => serversExporter.exportServers()}
-        >
+        <DropdownItem className="servers-dropdown__export-item" onClick={() => serversExporter.exportServers()}>
           Export servers
         </DropdownItem>
       </React.Fragment>
diff --git a/src/servers/prop-types/index.js b/src/servers/prop-types/index.js
index caec6f92..ddd725c7 100644
--- a/src/servers/prop-types/index.js
+++ b/src/servers/prop-types/index.js
@@ -1,10 +1,12 @@
 import PropTypes from 'prop-types';
 
 export const serverType = PropTypes.shape({
-  id: PropTypes.string,
-  name: PropTypes.string,
-  url: PropTypes.string,
-  apiKey: PropTypes.string,
+  id: PropTypes.string.isRequired,
+  name: PropTypes.string.isRequired,
+  url: PropTypes.string.isRequired,
+  apiKey: PropTypes.string.isRequired,
   version: PropTypes.string,
   printableVersion: PropTypes.string,
+  serverNotFound: PropTypes.bool,
+  serverNotReachable: PropTypes.bool,
 });
diff --git a/src/servers/reducers/selectedServer.js b/src/servers/reducers/selectedServer.js
index 861fba85..9d646458 100644
--- a/src/servers/reducers/selectedServer.js
+++ b/src/servers/reducers/selectedServer.js
@@ -21,20 +21,37 @@ const versionToSemVer = pipe(
 export const resetSelectedServer = createAction(RESET_SELECTED_SERVER);
 
 export const selectServer = ({ findServerById }, buildShlinkApiClient) => (serverId) => async (dispatch) => {
+  dispatch(resetSelectedServer());
   dispatch(resetShortUrlParams());
-
   const selectedServer = findServerById(serverId);
-  const { health } = buildShlinkApiClient(selectedServer);
-  const { version } = await health().catch(() => MIN_FALLBACK_VERSION);
 
-  dispatch({
-    type: SELECT_SERVER,
-    selectedServer: {
-      ...selectedServer,
-      version: versionToSemVer(version),
-      printableVersion: versionToPrintable(version),
-    },
-  });
+  if (!selectedServer) {
+    dispatch({
+      type: SELECT_SERVER,
+      selectedServer: { serverNotFound: true },
+    });
+
+    return;
+  }
+
+  try {
+    const { health } = buildShlinkApiClient(selectedServer);
+    const { version } = await health();
+
+    dispatch({
+      type: SELECT_SERVER,
+      selectedServer: {
+        ...selectedServer,
+        version: versionToSemVer(version),
+        printableVersion: versionToPrintable(version),
+      },
+    });
+  } catch (e) {
+    dispatch({
+      type: SELECT_SERVER,
+      selectedServer: { serverNotReachable: true },
+    });
+  }
 };
 
 export default handleActions({
diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js
index e4231e87..ba4b2bd6 100644
--- a/src/servers/services/provideServices.js
+++ b/src/servers/services/provideServices.js
@@ -18,7 +18,7 @@ const provideServices = (bottle, connect, withRouter) => {
 
   bottle.serviceFactory('ServersDropdown', ServersDropdown, 'ServersExporter');
   bottle.decorator('ServersDropdown', withRouter);
-  bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ], [ 'listServers', 'selectServer' ]));
+  bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ], [ 'listServers' ]));
 
   bottle.serviceFactory('DeleteServerModal', () => DeleteServerModal);
   bottle.decorator('DeleteServerModal', withRouter);