Merge pull request #227 from acelaya-forks/feature/edit-servers

Feature/edit servers

CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 * [#213](https://github.com/shlinkio/shlink-web-client/issues/213) The versions of both shlink-web-client and currently consumed Shlink server are now displayed in the footer.
 * [#221](https://github.com/shlinkio/shlink-web-client/issues/221) Improved how servers are handled, displaying meaningful errors when a not-found or a not-reachable server is tried to be loaded.
+* [#226](https://github.com/shlinkio/shlink-web-client/issues/226) Created servers can now be edited.
 
 #### Changed
 

package-lock.json

@@ -5340,7 +5340,7 @@
     },
     "discontinuous-range": {
       "version": "1.0.0",
-      "resolved": "https://registry.npmjs.org/discontinuous-range/-/discontinuous-range-1.0.0.tgz",
+      "resolved": "https://registry.yarnpkg.com/discontinuous-range/-/discontinuous-range-1.0.0.tgz",
       "integrity": "sha1-44Mx8IRLukm5qctxx3FYWqsbxlo=",
       "dev": true
     },
@@ -9346,7 +9346,7 @@
     },
     "is-subset": {
       "version": "0.1.1",
-      "resolved": "https://registry.npmjs.org/is-subset/-/is-subset-0.1.1.tgz",
+      "resolved": "https://registry.yarnpkg.com/is-subset/-/is-subset-0.1.1.tgz",
       "integrity": "sha1-ilkRfZMt4d4A8kX83TnOQ/HpOaY=",
       "dev": true
     },
@@ -10557,13 +10557,13 @@
     },
     "lodash.escape": {
       "version": "4.0.1",
-      "resolved": "https://registry.npmjs.org/lodash.escape/-/lodash.escape-4.0.1.tgz",
+      "resolved": "https://registry.yarnpkg.com/lodash.escape/-/lodash.escape-4.0.1.tgz",
       "integrity": "sha1-yQRGkMIeBClL6qUXcS/e0fqI3pg=",
       "dev": true
     },
     "lodash.flattendeep": {
       "version": "4.4.0",
-      "resolved": "https://registry.npmjs.org/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz",
+      "resolved": "https://registry.yarnpkg.com/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz",
       "integrity": "sha1-+wMJF/hqMTTlvJvsDWngAT3f7bI=",
       "dev": true
     },
@@ -10575,7 +10575,7 @@
     },
     "lodash.isequal": {
       "version": "4.5.0",
-      "resolved": "https://registry.npmjs.org/lodash.isequal/-/lodash.isequal-4.5.0.tgz",
+      "resolved": "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz",
       "integrity": "sha1-QVxEePK8wwEgwizhDtMib30+GOA=",
       "dev": true
     },
@@ -13817,7 +13817,7 @@
     },
     "railroad-diagrams": {
       "version": "1.0.0",
-      "resolved": "https://registry.npmjs.org/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz",
+      "resolved": "https://registry.yarnpkg.com/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz",
       "integrity": "sha1-635iZ1SN3t+4mcG5Dlc3RVnN234=",
       "dev": true
     },
@@ -14977,7 +14977,7 @@
     },
     "rst-selector-parser": {
       "version": "2.2.3",
-      "resolved": "https://registry.npmjs.org/rst-selector-parser/-/rst-selector-parser-2.2.3.tgz",
+      "resolved": "https://registry.yarnpkg.com/rst-selector-parser/-/rst-selector-parser-2.2.3.tgz",
       "integrity": "sha1-gbIw6i/MYGbInjRy3nlChdmwPZE=",
       "dev": true,
       "requires": {

src/App.js

@@ -3,14 +3,15 @@ import { Route, Switch } from 'react-router-dom';
 import './App.scss';
 import NotFound from './common/NotFound';
 
-const App = (MainHeader, Home, MenuLayout, CreateServer) => () => (
+const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer) => () => (
   <div className="container-fluid app-container">
     <MainHeader />
 
     <div className="app">
       <Switch>
-        <Route exact path="/server/create" component={CreateServer} />
         <Route exact path="/" component={Home} />
+        <Route exact path="/server/create" component={CreateServer} />
+        <Route exact path="/server/:serverId/edit" component={EditServer} />
         <Route path="/server/:serverId" component={MenuLayout} />
         <Route component={NotFound} />
       </Switch>

src/common/AsideMenu.js

@@ -1,4 +1,9 @@
-import { faList as listIcon, faLink as createIcon, faTags as tagsIcon } from '@fortawesome/free-solid-svg-icons';
+import {
+  faList as listIcon,
+  faLink as createIcon,
+  faTags as tagsIcon,
+  faPen as editIcon,
+} from '@fortawesome/free-solid-svg-icons';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import React from 'react';
 import { NavLink } from 'react-router-dom';
@@ -7,8 +12,13 @@ import classNames from 'classnames';
 import { serverType } from '../servers/prop-types';
 import './AsideMenu.scss';
 
-const AsideMenuItem = ({ children, to, ...rest }) => (
-  <NavLink className="aside-menu__item" activeClassName="aside-menu__item--selected" to={to} {...rest}>
+const AsideMenuItem = ({ children, to, className, ...rest }) => (
+  <NavLink
+    className={classNames('aside-menu__item', className)}
+    activeClassName="aside-menu__item--selected"
+    to={to}
+    {...rest}
+  >
     {children}
   </NavLink>
 );
@@ -16,6 +26,7 @@ const AsideMenuItem = ({ children, to, ...rest }) => (
 AsideMenuItem.propTypes = {
   children: PropTypes.node.isRequired,
   to: PropTypes.string.isRequired,
+  className: PropTypes.string,
 };
 
 const propTypes = {
@@ -48,8 +59,15 @@ const AsideMenu = (DeleteServerButton) => {
             <FontAwesomeIcon icon={tagsIcon} />
             <span className="aside-menu__item-text">Manage tags</span>
           </AsideMenuItem>
-
-          <DeleteServerButton className="aside-menu__item aside-menu__item--danger" server={selectedServer} />
+          <AsideMenuItem to={buildPath('/edit')} className="aside-menu__item--push">
+            <FontAwesomeIcon icon={editIcon} />
+            <span className="aside-menu__item-text">Edit this server</span>
+          </AsideMenuItem>
+          <DeleteServerButton
+            className="aside-menu__item aside-menu__item--danger"
+            textClassName="aside-menu__item-text"
+            server={selectedServer}
+          />
         </nav>
       </aside>
     );

src/common/AsideMenu.scss

@@ -67,6 +67,9 @@ $asideMenuMobileWidth: 280px;
 
 .aside-menu__item--danger {
   color: $dangerColor;
+}
+
+.aside-menu__item--push {
   margin-top: auto;
 }
 

src/common/MenuLayout.js

@@ -6,35 +6,23 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import classNames from 'classnames';
 import * as PropTypes from 'prop-types';
 import { serverType } from '../servers/prop-types';
-import Message from '../utils/Message';
+import { withSelectedServer } from '../servers/helpers/withSelectedServer';
 import NotFound from './NotFound';
 import './MenuLayout.scss';
 
 const propTypes = {
   match: PropTypes.object,
-  selectServer: PropTypes.func,
   location: PropTypes.object,
   selectedServer: serverType,
 };
 
 const MenuLayout = (TagsList, ShortUrls, AsideMenu, CreateShortUrl, ShortUrlVisits, ShlinkVersions, ServerError) => {
-  const MenuLayoutComp = ({ match, location, selectedServer, selectServer }) => {
+  const MenuLayoutComp = ({ match, location, selectedServer }) => {
     const [ showSideBar, setShowSidebar ] = useState(false);
     const { params: { serverId } } = match;
 
-    useEffect(() => {
-      selectServer(serverId);
-    }, [ serverId ]);
     useEffect(() => setShowSidebar(false), [ location ]);
 
-    if (!selectedServer) {
-      return <Message loading />;
-    }
-
-    if (selectedServer.serverNotFound) {
-      return <ServerError type="not-found" />;
-    }
-
     if (selectedServer.serverNotReachable) {
       return <ServerError type="not-reachable" />;
     }
@@ -91,7 +79,7 @@ const MenuLayout = (TagsList, ShortUrls, AsideMenu, CreateShortUrl, ShortUrlVisi
 
   MenuLayoutComp.propTypes = propTypes;
 
-  return MenuLayoutComp;
+  return withSelectedServer(MenuLayoutComp, ServerError);
 };
 
 export default MenuLayout;

src/container/index.js

@@ -26,7 +26,7 @@ const connect = (propsFromState, actionServiceNames = []) =>
     actionServiceNames.reduce(mapActionService, {})
   );
 
-bottle.serviceFactory('App', App, 'MainHeader', 'Home', 'MenuLayout', 'CreateServer');
+bottle.serviceFactory('App', App, 'MainHeader', 'Home', 'MenuLayout', 'CreateServer', 'EditServer');
 
 provideCommonServices(bottle, connect, withRouter);
 provideShortUrlsServices(bottle, connect);

src/servers/CreateServer.js

@@ -1,91 +1,56 @@
-import { assoc, dissoc, pipe } from 'ramda';
-import React from 'react';
+import React, { useEffect } from 'react';
 import { v4 as uuid } from 'uuid';
 import PropTypes from 'prop-types';
 import './CreateServer.scss';
+import { ServerForm } from './helpers/ServerForm';
 
 const SHOW_IMPORT_MSG_TIME = 4000;
+const propTypes = {
+  createServer: PropTypes.func,
+  history: PropTypes.shape({
+    push: PropTypes.func,
+  }),
+  resetSelectedServer: PropTypes.func,
+};
 
-const CreateServer = (ImportServersBtn, stateFlagTimeout) => class CreateServer extends React.Component {
-  static propTypes = {
-    createServer: PropTypes.func,
-    history: PropTypes.shape({
-      push: PropTypes.func,
-    }),
-    resetSelectedServer: PropTypes.func,
-  };
+const CreateServer = (ImportServersBtn, useStateFlagTimeout) => {
+  const CreateServerComp = ({ createServer, history: { push }, resetSelectedServer }) => {
+    const [ serversImported, setServersImported ] = useStateFlagTimeout(false, SHOW_IMPORT_MSG_TIME);
+    const handleSubmit = (serverData) => {
+      const id = uuid();
+      const server = { id, ...serverData };
 
-  state = {
-    name: '',
-    url: '',
-    apiKey: '',
-    serversImported: false,
-  };
+      createServer(server);
+      push(`/server/${id}/list-short-urls/1`);
+    };
 
-  handleSubmit = (e) => {
-    e.preventDefault();
-
-    const { createServer, history: { push } } = this.props;
-    const server = pipe(
-      assoc('id', uuid()),
-      dissoc('serversImported')
-    )(this.state);
-
-    createServer(server);
-    push(`/server/${server.id}/list-short-urls/1`);
-  };
-
-  componentDidMount() {
-    this.props.resetSelectedServer();
-  }
-
-  render() {
-    const renderInputGroup = (id, placeholder, type = 'text') => (
-      <div className="form-group row">
-        <label htmlFor={id} className="col-lg-1 col-md-2 col-form-label create-server__label">
-          {placeholder}:
-        </label>
-        <div className="col-lg-11 col-md-10">
-          <input
-            type={type}
-            className="form-control"
-            id={id}
-            placeholder={placeholder}
-            value={this.state[id]}
-            required
-            onChange={(e) => this.setState({ [id]: e.target.value })}
-          />
-        </div>
-      </div>
-    );
+    useEffect(() => {
+      resetSelectedServer();
+    }, []);
 
     return (
       <div className="create-server">
-        <form onSubmit={this.handleSubmit}>
-          {renderInputGroup('name', 'Name')}
-          {renderInputGroup('url', 'URL', 'url')}
-          {renderInputGroup('apiKey', 'API key')}
+        <ServerForm onSubmit={handleSubmit}>
+          <ImportServersBtn onImport={setServersImported} />
+          <button className="btn btn-outline-primary">Create server</button>
+        </ServerForm>
 
-          <div className="text-right">
-            <ImportServersBtn
-              onImport={() => stateFlagTimeout(this.setState.bind(this), 'serversImported', true, SHOW_IMPORT_MSG_TIME)}
-            />
-            <button className="btn btn-outline-primary">Create server</button>
-          </div>
-
-          {this.state.serversImported && (
-            <div className="row create-server__import-success-msg">
-              <div className="col-md-10 offset-md-1">
-                <div className="p-2 mt-3 bg-main text-white text-center">
-                  Servers properly imported. You can now select one from the list :)
-                </div>
+        {serversImported && (
+          <div className="row create-server__import-success-msg">
+            <div className="col-md-10 offset-md-1">
+              <div className="p-2 mt-3 bg-main text-white text-center">
+                Servers properly imported. You can now select one from the list :)
               </div>
             </div>
-          )}
-        </form>
+          </div>
+        )}
       </div>
     );
-  }
+  };
+
+  CreateServerComp.propTypes = propTypes;
+
+  return CreateServerComp;
 };
 
 export default CreateServer;

src/servers/DeleteServerButton.js

@@ -7,25 +7,22 @@ import { serverType } from './prop-types';
 const propTypes = {
   server: serverType,
   className: PropTypes.string,
+  textClassName: PropTypes.string,
+  children: PropTypes.node,
 };
 
 const DeleteServerButton = (DeleteServerModal) => {
-  const DeleteServerButtonComp = ({ server, className }) => {
+  const DeleteServerButtonComp = ({ server, className, children, textClassName }) => {
     const [ isModalOpen, setModalOpen ] = useState(false);
 
     return (
       <React.Fragment>
-        <span className={className} key="deleteServerBtn" onClick={() => setModalOpen(true)}>
-          <FontAwesomeIcon icon={deleteIcon} />
-          <span className="aside-menu__item-text">Remove this server</span>
+        <span className={className} onClick={() => setModalOpen(true)}>
+          {!children && <FontAwesomeIcon icon={deleteIcon} />}
+          <span className={textClassName}>{children || 'Remove this server'}</span>
         </span>
 
-        <DeleteServerModal
-          isOpen={isModalOpen}
-          toggle={() => setModalOpen(!isModalOpen)}
-          server={server}
-          key="deleteServerModal"
-        />
+        <DeleteServerModal server={server} isOpen={isModalOpen} toggle={() => setModalOpen(!isModalOpen)} />
       </React.Fragment>
     );
   };

src/servers/EditServer.js

@@ -0,0 +1,34 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { ServerForm } from './helpers/ServerForm';
+import { withSelectedServer } from './helpers/withSelectedServer';
+import { serverType } from './prop-types';
+
+const propTypes = {
+  editServer: PropTypes.func,
+  selectedServer: serverType,
+  history: PropTypes.shape({
+    push: PropTypes.func,
+  }),
+};
+
+export const EditServer = (ServerError) => {
+  const EditServerComp = ({ editServer, selectedServer, history: { push } }) => {
+    const handleSubmit = (serverData) => {
+      editServer(selectedServer.id, serverData);
+      push(`/server/${selectedServer.id}/list-short-urls/1`);
+    };
+
+    return (
+      <div className="create-server">
+        <ServerForm initialValues={selectedServer} onSubmit={handleSubmit}>
+          <button className="btn btn-outline-primary">Save</button>
+        </ServerForm>
+      </div>
+    );
+  };
+
+  EditServerComp.propTypes = propTypes;
+
+  return withSelectedServer(EditServerComp, ServerError);
+};

src/servers/ServersListGroup.js

@@ -26,7 +26,9 @@ ServerListItem.propTypes = {
 
 const ServersListGroup = ({ servers, children }) => (
   <React.Fragment>
-    <h5>{children}</h5>
+    <div className="container">
+      <h5>{children}</h5>
+    </div>
     {servers.length > 0 && (
       <ListGroup className="servers-list__list-group mt-md-3">
         {servers.map(({ id, name }) => <ServerListItem key={id} id={id} name={name} />)}

src/servers/helpers/ServerError.js

@@ -3,31 +3,48 @@ import PropTypes from 'prop-types';
 import { Link } from 'react-router-dom';
 import Message from '../../utils/Message';
 import ServersListGroup from '../ServersListGroup';
+import { serverType } from '../prop-types';
 import './ServerError.scss';
 
 const propTypes = {
   servers: PropTypes.object,
+  selectedServer: serverType,
   type: PropTypes.oneOf([ 'not-found', 'not-reachable' ]).isRequired,
 };
 
-export const ServerError = ({ type, servers: { list } }) => (
-  <div className="server-error-container flex-column">
-    <div className="row w-100 mb-3 mb-md-5">
-      <Message type="error">
-        {type === 'not-found' && 'Could not find this Shlink server.'}
-        {type === 'not-reachable' && (
-          <React.Fragment>
-            <p>Oops! Could not connect to this Shlink server.</p>
-            Make sure you have internet connection, and the server is properly configured and on-line.
-          </React.Fragment>
-        )}
-      </Message>
-    </div>
-    <ServersListGroup servers={Object.values(list)}>
-      These are the {type === 'not-reachable' ? 'other' : ''} Shlink servers currently configured. Choose one of
-      them or <Link to="/server/create">add a new one</Link>.
-    </ServersListGroup>
-  </div>
-);
+export const ServerError = (DeleteServerButton) => {
+  const ServerErrorComp = ({ type, servers: { list }, selectedServer }) => (
+    <div className="server-error__container flex-column">
+      <div className="row w-100 mb-3 mb-md-5">
+        <Message type="error">
+          {type === 'not-found' && 'Could not find this Shlink server.'}
+          {type === 'not-reachable' && (
+            <React.Fragment>
+              <p>Oops! Could not connect to this Shlink server.</p>
+              Make sure you have internet connection, and the server is properly configured and on-line.
+            </React.Fragment>
+          )}
+        </Message>
+      </div>
 
-ServerError.propTypes = propTypes;
+      <ServersListGroup servers={Object.values(list)}>
+        These are the Shlink servers currently configured. Choose one of
+        them or <Link to="/server/create">add a new one</Link>.
+      </ServersListGroup>
+
+      {type === 'not-reachable' && (
+        <div className="container mt-3 mt-md-5">
+          <h5>
+            Alternatively, if you think you may have miss-configured this server, you
+            can <DeleteServerButton server={selectedServer} className="server-error__delete-btn">remove it</DeleteServerButton> or&nbsp;
+            <Link to={`/server/${selectedServer.id}/edit`}>edit it</Link>.
+          </h5>
+        </div>
+      )}
+    </div>
+  );
+
+  ServerErrorComp.propTypes = propTypes;
+
+  return ServerErrorComp;
+};

src/servers/helpers/ServerError.scss

@@ -1,6 +1,17 @@
-.server-error-container {
+@import '../../utils/base';
+
+.server-error__container {
   text-align: center;
   display: flex;
   align-items: center;
   justify-content: center;
 }
+
+.server-error__delete-btn {
+  color: $dangerColor;
+  cursor: pointer;
+}
+
+.server-error__delete-btn:hover {
+  text-decoration: underline;
+}

src/servers/helpers/ServerForm.js

@@ -0,0 +1,41 @@
+import React, { useEffect, useState } from 'react';
+import PropTypes from 'prop-types';
+import { HorizontalFormGroup } from '../../utils/HorizontalFormGroup';
+
+const propTypes = {
+  onSubmit: PropTypes.func.isRequired,
+  initialValues: PropTypes.shape({
+    name: PropTypes.string.isRequired,
+    url: PropTypes.string.isRequired,
+    apiKey: PropTypes.string.isRequired,
+  }),
+  children: PropTypes.node.isRequired,
+};
+
+export const ServerForm = ({ onSubmit, initialValues, children }) => {
+  const [ name, setName ] = useState('');
+  const [ url, setUrl ] = useState('');
+  const [ apiKey, setApiKey ] = useState('');
+  const handleSubmit = (e) => {
+    e.preventDefault();
+    onSubmit({ name, url, apiKey });
+  };
+
+  useEffect(() => {
+    initialValues && setName(initialValues.name);
+    initialValues && setUrl(initialValues.url);
+    initialValues && setApiKey(initialValues.apiKey);
+  }, [ initialValues ]);
+
+  return (
+    <form onSubmit={handleSubmit}>
+      <HorizontalFormGroup value={name} onChange={setName}>Name</HorizontalFormGroup>
+      <HorizontalFormGroup type="url" value={url} onChange={setUrl}>URL</HorizontalFormGroup>
+      <HorizontalFormGroup value={apiKey} onChange={setApiKey}>API key</HorizontalFormGroup>
+
+      <div className="text-right">{children}</div>
+    </form>
+  );
+};
+
+ServerForm.propTypes = propTypes;

src/servers/helpers/withSelectedServer.js

@@ -0,0 +1,35 @@
+import React, { useEffect } from 'react';
+import PropTypes from 'prop-types';
+import Message from '../../utils/Message';
+import { serverType } from '../prop-types';
+
+const propTypes = {
+  selectServer: PropTypes.func,
+  selectedServer: serverType,
+  match: PropTypes.object,
+};
+
+export const withSelectedServer = (WrappedComponent, ServerError) => {
+  const Component = (props) => {
+    const { selectServer, selectedServer, match } = props;
+    const { params: { serverId } } = match;
+
+    useEffect(() => {
+      selectServer(serverId);
+    }, [ serverId ]);
+
+    if (!selectedServer) {
+      return <Message loading />;
+    }
+
+    if (selectedServer.serverNotFound) {
+      return <ServerError type="not-found" />;
+    }
+
+    return <WrappedComponent {...props} />;
+  };
+
+  Component.propTypes = propTypes;
+
+  return Component;
+};

src/servers/prop-types/index.js

@@ -7,18 +7,14 @@ const regularServerType = PropTypes.shape({
   apiKey: PropTypes.string,
   version: PropTypes.string,
   printableVersion: PropTypes.string,
+  serverNotReachable: PropTypes.bool,
 });
 
 const notFoundServerType = PropTypes.shape({
   serverNotFound: PropTypes.bool.isRequired,
 });
 
-const notReachableServerType = PropTypes.shape({
-  serverNotReachable: PropTypes.bool.isRequired,
-});
-
 export const serverType = PropTypes.oneOfType([
   regularServerType,
   notFoundServerType,
-  notReachableServerType,
 ]);

src/servers/reducers/selectedServer.js

@@ -49,7 +49,7 @@ export const selectServer = ({ findServerById }, buildShlinkApiClient) => (serve
   } catch (e) {
     dispatch({
       type: SELECT_SERVER,
-      selectedServer: { serverNotReachable: true },
+      selectedServer: { ...selectedServer, serverNotReachable: true },
     });
   }
 };

src/servers/reducers/server.js

@@ -52,6 +52,8 @@ export const listServers = ({ listServers, createServers }, { get }) => () => as
 
 export const createServer = ({ createServer }, listServersAction) => pipe(createServer, listServersAction);
 
+export const editServer = ({ editServer }, listServersAction) => pipe(editServer, listServersAction);
+
 export const deleteServer = ({ deleteServer }, listServersAction) => pipe(deleteServer, listServersAction);
 
 export const createServers = ({ createServers }, listServersAction) => pipe(

src/servers/services/ServersService.js

@@ -25,4 +25,14 @@ export default class ServersService {
 
   deleteServer = ({ id }) =>
     this.storage.set(SERVERS_STORAGE_KEY, dissoc(id, this.listServers()));
+
+  editServer = (id, serverData) => {
+    const allServers = this.listServers();
+
+    if (!allServers[id]) {
+      return;
+    }
+
+    this.storage.set(SERVERS_STORAGE_KEY, assoc(id, { ...allServers[id], ...serverData }, allServers));
+  }
 }

src/servers/services/provideServices.js

@@ -3,9 +3,10 @@ import CreateServer from '../CreateServer';
 import ServersDropdown from '../ServersDropdown';
 import DeleteServerModal from '../DeleteServerModal';
 import DeleteServerButton from '../DeleteServerButton';
+import { EditServer } from '../EditServer';
 import ImportServersBtn from '../helpers/ImportServersBtn';
 import { resetSelectedServer, selectServer } from '../reducers/selectedServer';
-import { createServer, createServers, deleteServer, listServers } from '../reducers/server';
+import { createServer, createServers, deleteServer, editServer, listServers } from '../reducers/server';
 import ForServerVersion from '../helpers/ForServerVersion';
 import { ServerError } from '../helpers/ServerError';
 import ServersImporter from './ServersImporter';
@@ -14,9 +15,12 @@ import ServersExporter from './ServersExporter';
 
 const provideServices = (bottle, connect, withRouter) => {
   // Components
-  bottle.serviceFactory('CreateServer', CreateServer, 'ImportServersBtn', 'stateFlagTimeout');
+  bottle.serviceFactory('CreateServer', CreateServer, 'ImportServersBtn', 'useStateFlagTimeout');
   bottle.decorator('CreateServer', connect([ 'selectedServer' ], [ 'createServer', 'resetSelectedServer' ]));
 
+  bottle.serviceFactory('EditServer', EditServer, 'ServerError');
+  bottle.decorator('EditServer', connect([ 'selectedServer' ], [ 'editServer', 'selectServer' ]));
+
   bottle.serviceFactory('ServersDropdown', ServersDropdown, 'ServersExporter');
   bottle.decorator('ServersDropdown', withRouter);
   bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ], [ 'listServers' ]));
@@ -33,8 +37,8 @@ const provideServices = (bottle, connect, withRouter) => {
   bottle.serviceFactory('ForServerVersion', () => ForServerVersion);
   bottle.decorator('ForServerVersion', connect([ 'selectedServer' ]));
 
-  bottle.serviceFactory('ServerError', () => ServerError);
-  bottle.decorator('ServerError', connect([ 'servers' ]));
+  bottle.serviceFactory('ServerError', ServerError, 'DeleteServerButton');
+  bottle.decorator('ServerError', connect([ 'servers', 'selectedServer' ]));
 
   // Services
   bottle.constant('csvjson', csvjson);
@@ -47,6 +51,7 @@ const provideServices = (bottle, connect, withRouter) => {
   bottle.serviceFactory('createServer', createServer, 'ServersService', 'listServers');
   bottle.serviceFactory('createServers', createServers, 'ServersService', 'listServers');
   bottle.serviceFactory('deleteServer', deleteServer, 'ServersService', 'listServers');
+  bottle.serviceFactory('editServer', editServer, 'ServersService', 'listServers');
   bottle.serviceFactory('listServers', listServers, 'ServersService', 'axios');
 
   bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer);

src/utils/HorizontalFormGroup.js

@@ -0,0 +1,32 @@
+import React from 'react';
+import { v4 as uuid } from 'uuid';
+import PropTypes from 'prop-types';
+
+const propTypes = {
+  children: PropTypes.node.isRequired,
+  value: PropTypes.string.isRequired,
+  onChange: PropTypes.func.isRequired,
+  id: PropTypes.string,
+  type: PropTypes.string,
+  required: PropTypes.bool,
+};
+
+export const HorizontalFormGroup = ({ children, value, onChange, id = uuid(), type = 'text', required = true }) => (
+  <div className="form-group row">
+    <label htmlFor={id} className="col-lg-1 col-md-2 col-form-label create-server__label">
+      {children}:
+    </label>
+    <div className="col-lg-11 col-md-10">
+      <input
+        className="form-control"
+        type={type}
+        id={id}
+        value={value}
+        required={required}
+        onChange={(e) => onChange(e.target.value)}
+      />
+    </div>
+  </div>
+);
+
+HorizontalFormGroup.propTypes = propTypes;

test/App.test.js

@@ -9,7 +9,7 @@ describe('<App />', () => {
   const MainHeader = () => '';
 
   beforeEach(() => {
-    const App = appFactory(MainHeader, identity, identity, identity);
+    const App = appFactory(MainHeader, identity, identity, identity, identity);
 
     wrapper = shallow(<App />);
   });
@@ -20,13 +20,14 @@ describe('<App />', () => {
   it('renders app main routes', () => {
     const routes = wrapper.find(Route);
     const expectedPaths = [
-      '/server/create',
       '/',
+      '/server/create',
+      '/server/:serverId/edit',
       '/server/:serverId',
     ];
 
     expect.assertions(expectedPaths.length + 1);
-    expect(routes).toHaveLength(4);
+    expect(routes).toHaveLength(expectedPaths.length + 1);
     expectedPaths.forEach((path, index) => {
       expect(routes.at(index).prop('path')).toEqual(path);
     });

test/common/AsideMenu.test.js

@@ -16,7 +16,7 @@ describe('<AsideMenu />', () => {
   it('contains links to different sections', () => {
     const links = wrapped.find('[to]');
 
-    expect(links).toHaveLength(3);
+    expect(links).toHaveLength(4);
     links.forEach((link) => expect(link.prop('to')).toContain('abc123'));
   });
 

test/servers/CreateServer.test.js

@@ -2,6 +2,7 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import { identity } from 'ramda';
 import createServerConstruct from '../../src/servers/CreateServer';
+import { ServerForm } from '../../src/servers/helpers/ServerForm';
 
 describe('<CreateServer />', () => {
   let wrapper;
@@ -10,53 +11,41 @@ describe('<CreateServer />', () => {
   const historyMock = {
     push: jest.fn(),
   };
-
-  beforeEach(() => {
-    createServerMock.mockReset();
-
-    const CreateServer = createServerConstruct(ImportServersBtn);
+  const createWrapper = (serversImported = false) => {
+    const CreateServer = createServerConstruct(ImportServersBtn, () => [ serversImported, () => '' ]);
 
     wrapper = shallow(
       <CreateServer createServer={createServerMock} resetSelectedServer={identity} history={historyMock} />
     );
+
+    return wrapper;
+  };
+
+  afterEach(() => {
+    jest.resetAllMocks();
+    wrapper && wrapper.unmount();
   });
-  afterEach(() => wrapper.unmount());
 
   it('renders components', () => {
-    expect(wrapper.find('#name')).toHaveLength(1);
-    expect(wrapper.find('#url')).toHaveLength(1);
-    expect(wrapper.find('#apiKey')).toHaveLength(1);
-    expect(wrapper.find(ImportServersBtn)).toHaveLength(1);
+    const wrapper = createWrapper();
+
+    expect(wrapper.find(ServerForm)).toHaveLength(1);
     expect(wrapper.find('.create-server__import-success-msg')).toHaveLength(0);
   });
 
   it('shows success message when imported is true', () => {
-    wrapper.setState({ serversImported: true });
+    const wrapper = createWrapper(true);
+
     expect(wrapper.find('.create-server__import-success-msg')).toHaveLength(1);
   });
 
   it('creates server and redirects to it when form is submitted', () => {
-    const form = wrapper.find('form');
+    const wrapper = createWrapper();
+    const form = wrapper.find(ServerForm);
 
-    form.simulate('submit', { preventDefault() {
-      return '';
-    } });
+    form.simulate('submit', {});
 
     expect(createServerMock).toHaveBeenCalledTimes(1);
     expect(historyMock.push).toHaveBeenCalledTimes(1);
   });
-
-  it('updates state when inputs are changed', () => {
-    const nameInput = wrapper.find('#name');
-    const urlInput = wrapper.find('#url');
-    const apiKeyInput = wrapper.find('#apiKey');
-
-    nameInput.simulate('change', { target: { value: 'the_name' } });
-    urlInput.simulate('change', { target: { value: 'the_url' } });
-    apiKeyInput.simulate('change', { target: { value: 'the_api_key' } });
-
-    expect(wrapper.state('name')).toEqual('the_name');
-    expect(wrapper.state('url')).toEqual('the_url');
-    expect(wrapper.state('apiKey')).toEqual('the_api_key');
-  });
 });

test/servers/EditServer.test.js

@@ -0,0 +1,52 @@
+import React from 'react';
+import { mount } from 'enzyme';
+import { EditServer as editServerConstruct } from '../../src/servers/EditServer';
+import { ServerForm } from '../../src/servers/helpers/ServerForm';
+
+describe('<EditServer />', () => {
+  let wrapper;
+  const ServerError = jest.fn();
+  const editServerMock = jest.fn();
+  const historyMock = { push: jest.fn() };
+  const match = {
+    params: { serverId: 'abc123' },
+  };
+  const selectedServer = {
+    id: 'abc123',
+    name: 'name',
+    url: 'url',
+    apiKey: 'apiKey',
+  };
+
+  beforeEach(() => {
+    const EditServer = editServerConstruct(ServerError);
+
+    wrapper = mount(
+      <EditServer
+        editServer={editServerMock}
+        history={historyMock}
+        match={match}
+        selectedServer={selectedServer}
+        selectServer={jest.fn()}
+      />
+    );
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+    wrapper && wrapper.unmount();
+  });
+
+  it('renders components', () => {
+    expect(wrapper.find(ServerForm)).toHaveLength(1);
+  });
+
+  it('edits server and redirects to it when form is submitted', () => {
+    const form = wrapper.find(ServerForm);
+
+    form.simulate('submit', {});
+
+    expect(editServerMock).toHaveBeenCalledTimes(1);
+    expect(historyMock.push).toHaveBeenCalledTimes(1);
+  });
+});

test/servers/helpers/ServerError.test.js

@@ -1,35 +1,49 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 import { BrowserRouter } from 'react-router-dom';
-import { ServerError } from '../../../src/servers/helpers/ServerError';
+import { ServerError as createServerError } from '../../../src/servers/helpers/ServerError';
 
 describe('<ServerError />', () => {
   let wrapper;
+  const selectedServer = { id: '' };
+  const ServerError = createServerError(() => '');
 
   afterEach(() => wrapper && wrapper.unmount());
 
   it.each([
     [
       'not-found',
-      [ 'Could not find this Shlink server.' ],
-      'These are the Shlink servers',
+      {
+        'Could not find this Shlink server.': true,
+        'Oops! Could not connect to this Shlink server.': false,
+        'Make sure you have internet connection, and the server is properly configured and on-line.': false,
+        'Alternatively, if you think you may have miss-configured this server': false,
+      },
     ],
     [
       'not-reachable',
-      [
-        'Oops! Could not connect to this Shlink server.',
-        'Make sure you have internet connection, and the server is properly configured and on-line.',
-      ],
-      'These are the other Shlink servers',
+      {
+        'Could not find this Shlink server.': false,
+        'Oops! Could not connect to this Shlink server.': true,
+        'Make sure you have internet connection, and the server is properly configured and on-line.': true,
+        'Alternatively, if you think you may have miss-configured this server': true,
+      },
     ],
-  ])('renders expected information based on type', (type, expectedTitleParts, expectedBody) => {
-    wrapper = shallow(<BrowserRouter><ServerError type={type} servers={{ list: [] }} /></BrowserRouter>);
+  ])('renders expected information for type "%s"', (type, textsToFind) => {
+    wrapper = shallow(
+      <BrowserRouter>
+        <ServerError type={type} servers={{ list: [] }} selectedServer={selectedServer} />
+      </BrowserRouter>
+    );
     const wrapperText = wrapper.html();
-    const textsToFind = [ ...expectedTitleParts, ...expectedBody ];
+    const textPairs = Object.entries(textsToFind);
 
-    expect.assertions(textsToFind.length);
-    textsToFind.forEach((text) => {
-      expect(wrapperText).toContain(text);
+    textPairs.forEach(([ text, shouldBeFound ]) => {
+      if (shouldBeFound) {
+        expect(wrapperText).toContain(text);
+      } else {
+        expect(wrapperText).not.toContain(text);
+      }
     });
   });
 });

test/servers/helpers/ServerForm.test.js

@@ -0,0 +1,33 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { ServerForm } from '../../../src/servers/helpers/ServerForm';
+import { HorizontalFormGroup } from '../../../src/utils/HorizontalFormGroup';
+
+describe('<ServerForm />', () => {
+  let wrapper;
+  const onSubmit = jest.fn();
+
+  beforeEach(() => {
+    wrapper = shallow(<ServerForm onSubmit={onSubmit}><span>Something</span></ServerForm>);
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+    wrapper && wrapper.unmount();
+  });
+
+  it('renders components', () => {
+    expect(wrapper.find(HorizontalFormGroup)).toHaveLength(3);
+    expect(wrapper.find('span')).toHaveLength(1);
+  });
+
+  it('invokes submit callback when submit event is triggered', () => {
+    const form = wrapper.find('form');
+    const preventDefault = jest.fn();
+
+    form.simulate('submit', { preventDefault });
+
+    expect(preventDefault).toHaveBeenCalled();
+    expect(onSubmit).toHaveBeenCalled();
+  });
+});

test/servers/reducers/selectedServer.test.js

@@ -72,7 +72,7 @@ describe('selectedServerReducer', () => {
     });
 
     it('dispatches error when health endpoint fails', async () => {
-      const expectedSelectedServer = { serverNotReachable: true };
+      const expectedSelectedServer = { ...selectedServer, serverNotReachable: true };
 
       apiClientMock.health.mockRejectedValue({});
 

test/servers/reducers/server.test.js

@@ -4,7 +4,7 @@ import reducer, {
   deleteServer,
   listServers,
   createServers,
-  FETCH_SERVERS, FETCH_SERVERS_START,
+  FETCH_SERVERS, FETCH_SERVERS_START, editServer,
 } from '../../../src/servers/reducers/server';
 
 describe('serverReducer', () => {
@@ -16,6 +16,7 @@ describe('serverReducer', () => {
   const ServersServiceMock = {
     listServers: jest.fn(() => list),
     createServer: jest.fn(),
+    editServer: jest.fn(),
     deleteServer: jest.fn(),
     createServers: jest.fn(),
   };
@@ -41,6 +42,7 @@ describe('serverReducer', () => {
         expect(dispatch).toHaveBeenNthCalledWith(2, expectedFetchServersResult);
         expect(ServersServiceMock.listServers).toHaveBeenCalledTimes(1);
         expect(ServersServiceMock.createServer).not.toHaveBeenCalled();
+        expect(ServersServiceMock.editServer).not.toHaveBeenCalled();
         expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServers).not.toHaveBeenCalled();
         expect(axios.get).not.toHaveBeenCalled();
@@ -91,6 +93,7 @@ describe('serverReducer', () => {
         expect(dispatch).toHaveBeenNthCalledWith(2, { type: FETCH_SERVERS, list: expectedList });
         expect(NoListServersServiceMock.listServers).toHaveBeenCalledTimes(1);
         expect(NoListServersServiceMock.createServer).not.toHaveBeenCalled();
+        expect(NoListServersServiceMock.editServer).not.toHaveBeenCalled();
         expect(NoListServersServiceMock.deleteServer).not.toHaveBeenCalled();
         expect(NoListServersServiceMock.createServers).toHaveBeenCalledTimes(1);
         expect(axios.get).toHaveBeenCalledTimes(1);
@@ -103,9 +106,25 @@ describe('serverReducer', () => {
         const result = createServer(ServersServiceMock, () => expectedFetchServersResult)(serverToCreate);
 
         expect(result).toEqual(expectedFetchServersResult);
+        expect(ServersServiceMock.listServers).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServer).toHaveBeenCalledTimes(1);
         expect(ServersServiceMock.createServer).toHaveBeenCalledWith(serverToCreate);
+        expect(ServersServiceMock.editServer).not.toHaveBeenCalled();
+        expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled();
+        expect(ServersServiceMock.createServers).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('editServer', () => {
+      it('edits existing server and then fetches servers again', () => {
+        const serverToEdit = { name: 'edited' };
+        const result = editServer(ServersServiceMock, () => expectedFetchServersResult)('123', serverToEdit);
+
+        expect(result).toEqual(expectedFetchServersResult);
         expect(ServersServiceMock.listServers).not.toHaveBeenCalled();
+        expect(ServersServiceMock.createServer).not.toHaveBeenCalled();
+        expect(ServersServiceMock.editServer).toHaveBeenCalledTimes(1);
+        expect(ServersServiceMock.editServer).toHaveBeenCalledWith('123', serverToEdit);
         expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServers).not.toHaveBeenCalled();
       });
@@ -120,12 +139,13 @@ describe('serverReducer', () => {
         expect(ServersServiceMock.listServers).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServer).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServers).not.toHaveBeenCalled();
+        expect(ServersServiceMock.editServer).not.toHaveBeenCalled();
         expect(ServersServiceMock.deleteServer).toHaveBeenCalledTimes(1);
         expect(ServersServiceMock.deleteServer).toHaveBeenCalledWith(serverToDelete);
       });
     });
 
-    describe('createServer', () => {
+    describe('createServers', () => {
       it('creates multiple servers and then fetches servers again', () => {
         const serversToCreate = values(list);
         const result = createServers(ServersServiceMock, () => expectedFetchServersResult)(serversToCreate);
@@ -133,9 +153,10 @@ describe('serverReducer', () => {
         expect(result).toEqual(expectedFetchServersResult);
         expect(ServersServiceMock.listServers).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServer).not.toHaveBeenCalled();
+        expect(ServersServiceMock.editServer).not.toHaveBeenCalled();
+        expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled();
         expect(ServersServiceMock.createServers).toHaveBeenCalledTimes(1);
         expect(ServersServiceMock.createServers).toHaveBeenCalledWith(serversToCreate);
-        expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled();
       });
     });
   });

test/servers/services/ServersService.test.js

@@ -5,15 +5,19 @@ describe('ServersService', () => {
     abc123: { id: 'abc123' },
     def456: { id: 'def456' },
   };
-  const createStorageMock = (returnValue) => ({
-    set: jest.fn(),
-    get: jest.fn(() => returnValue),
-  });
+  const createService = (withServers = true) => {
+    const storageMock = {
+      set: jest.fn(),
+      get: jest.fn(() => withServers ? servers : undefined),
+    };
+    const service = new ServersService(storageMock);
+
+    return [ service, storageMock ];
+  };
 
   describe('listServers', () => {
     it('returns an empty object when servers are not found in storage', () => {
-      const storageMock = createStorageMock();
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService(false);
 
       const result = service.listServers();
 
@@ -23,8 +27,7 @@ describe('ServersService', () => {
     });
 
     it('returns value from storage when found', () => {
-      const storageMock = createStorageMock(servers);
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService();
 
       const result = service.listServers();
 
@@ -36,8 +39,7 @@ describe('ServersService', () => {
 
   describe('findServerById', () => {
     it('returns undefined when requested server is not found', () => {
-      const storageMock = createStorageMock(servers);
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService();
 
       const result = service.findServerById('ghi789');
 
@@ -47,8 +49,7 @@ describe('ServersService', () => {
     });
 
     it('returns server from list when found', () => {
-      const storageMock = createStorageMock(servers);
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService();
 
       const result = service.findServerById('abc123');
 
@@ -60,8 +61,7 @@ describe('ServersService', () => {
 
   describe('createServer', () => {
     it('adds one server to the list', () => {
-      const storageMock = createStorageMock(servers);
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService();
 
       service.createServer({ id: 'ghi789' });
 
@@ -77,8 +77,7 @@ describe('ServersService', () => {
 
   describe('createServers', () => {
     it('adds multiple servers to the list', () => {
-      const storageMock = createStorageMock(servers);
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService();
 
       service.createServers([{ id: 'ghi789' }, { id: 'jkl123' }]);
 
@@ -95,8 +94,7 @@ describe('ServersService', () => {
 
   describe('deleteServer', () => {
     it('removes one server from the list', () => {
-      const storageMock = createStorageMock(servers);
-      const service = new ServersService(storageMock);
+      const [ service, storageMock ] = createService();
 
       service.deleteServer({ id: 'abc123' });
 
@@ -107,4 +105,26 @@ describe('ServersService', () => {
       });
     });
   });
+
+  describe('editServer', () => {
+    it('dos nothing is provided server does not exist', () => {
+      const [ service, storageMock ] = createService();
+
+      service.editServer('notFound', {});
+
+      expect(storageMock.set).not.toHaveBeenCalled();
+    });
+
+    it('updates the list with provided server data', () => {
+      const [ service, storageMock ] = createService();
+      const serverData = { name: 'foo', apiKey: 'bar' };
+
+      service.editServer('abc123', serverData);
+
+      expect(storageMock.set).toHaveBeenCalledWith(expect.anything(), {
+        abc123: { id: 'abc123', ...serverData },
+        def456: { id: 'def456' },
+      });
+    });
+  });
 });