From f44ec42f51bb4bab9ddddfdc0b6befa559460d2c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 09:17:33 +0100 Subject: [PATCH 01/12] Added links to delete and edit the server when a server could not be reached --- package-lock.json | 14 +++--- src/App.js | 3 +- src/common/AsideMenu.js | 6 ++- src/servers/DeleteServerButton.js | 17 +++----- src/servers/ServersListGroup.js | 4 +- src/servers/helpers/ServerError.js | 57 ++++++++++++++++--------- src/servers/helpers/ServerError.scss | 11 ++++- src/servers/prop-types/index.js | 6 +-- src/servers/reducers/selectedServer.js | 2 +- src/servers/services/provideServices.js | 4 +- 10 files changed, 75 insertions(+), 49 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7945df60..175f0e87 100644 --- a/package-lock.json +++ b/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": { diff --git a/src/App.js b/src/App.js index ba40b7e8..5547e1e3 100644 --- a/src/App.js +++ b/src/App.js @@ -9,8 +9,9 @@ const App = (MainHeader, Home, MenuLayout, CreateServer) => () => (
- + + diff --git a/src/common/AsideMenu.js b/src/common/AsideMenu.js index 56e7d10c..433163ba 100644 --- a/src/common/AsideMenu.js +++ b/src/common/AsideMenu.js @@ -49,7 +49,11 @@ const AsideMenu = (DeleteServerButton) => { Manage tags - + ); diff --git a/src/servers/DeleteServerButton.js b/src/servers/DeleteServerButton.js index 8d0caaff..e50610bc 100644 --- a/src/servers/DeleteServerButton.js +++ b/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 ( - setModalOpen(true)}> - - Remove this server + setModalOpen(true)}> + {!children && } + {children || 'Remove this server'} - setModalOpen(!isModalOpen)} - server={server} - key="deleteServerModal" - /> + setModalOpen(!isModalOpen)} /> ); }; diff --git a/src/servers/ServersListGroup.js b/src/servers/ServersListGroup.js index 476922c1..55f8784a 100644 --- a/src/servers/ServersListGroup.js +++ b/src/servers/ServersListGroup.js @@ -26,7 +26,9 @@ ServerListItem.propTypes = { const ServersListGroup = ({ servers, children }) => ( -
{children}
+
+
{children}
+
{servers.length > 0 && ( {servers.map(({ id, name }) => )} diff --git a/src/servers/helpers/ServerError.js b/src/servers/helpers/ServerError.js index 96f1d449..7389d394 100644 --- a/src/servers/helpers/ServerError.js +++ b/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 } }) => ( -
-
- - {type === 'not-found' && 'Could not find this Shlink server.'} - {type === '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 {type === 'not-reachable' ? 'other' : ''} Shlink servers currently configured. Choose one of - them or add a new one. - -
-); +export const ServerError = (DeleteServerButton) => { + const ServerErrorComp = ({ type, servers: { list }, selectedServer }) => ( +
+
+ + {type === 'not-found' && 'Could not find this Shlink server.'} + {type === '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. + + )} + +
-ServerError.propTypes = propTypes; + + These are the Shlink servers currently configured. Choose one of + them or add a new one. + + + {type === 'not-reachable' && ( +
+
+ Alternatively, if you think you may have miss-configured this server, you + can remove it or  + edit it. +
+
+ )} +
+ ); + + ServerErrorComp.propTypes = propTypes; + + return ServerErrorComp; +}; diff --git a/src/servers/helpers/ServerError.scss b/src/servers/helpers/ServerError.scss index 073758ae..a4d991b9 100644 --- a/src/servers/helpers/ServerError.scss +++ b/src/servers/helpers/ServerError.scss @@ -1,6 +1,15 @@ -.server-error-container { +.server-error__container { text-align: center; display: flex; align-items: center; justify-content: center; } + +.server-error__delete-btn { + color: #dc3545; + cursor: pointer; +} + +.server-error__delete-btn:hover { + text-decoration: underline; +} diff --git a/src/servers/prop-types/index.js b/src/servers/prop-types/index.js index 4bdb7355..221d946a 100644 --- a/src/servers/prop-types/index.js +++ b/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, ]); diff --git a/src/servers/reducers/selectedServer.js b/src/servers/reducers/selectedServer.js index 9d646458..75b1237a 100644 --- a/src/servers/reducers/selectedServer.js +++ b/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 }, }); } }; diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index 9991d15b..5533372f 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -33,8 +33,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); From 8223f0fd6451f5c53a57633e0b1f5ddd45f92b51 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 09:43:42 +0100 Subject: [PATCH 02/12] Undone weird changes in package lock file --- package-lock.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index 175f0e87..7945df60 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5340,7 +5340,7 @@ }, "discontinuous-range": { "version": "1.0.0", - "resolved": "https://registry.yarnpkg.com/discontinuous-range/-/discontinuous-range-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/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.yarnpkg.com/is-subset/-/is-subset-0.1.1.tgz", + "resolved": "https://registry.npmjs.org/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.yarnpkg.com/lodash.escape/-/lodash.escape-4.0.1.tgz", + "resolved": "https://registry.npmjs.org/lodash.escape/-/lodash.escape-4.0.1.tgz", "integrity": "sha1-yQRGkMIeBClL6qUXcS/e0fqI3pg=", "dev": true }, "lodash.flattendeep": { "version": "4.4.0", - "resolved": "https://registry.yarnpkg.com/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz", + "resolved": "https://registry.npmjs.org/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.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz", + "resolved": "https://registry.npmjs.org/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.yarnpkg.com/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/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.yarnpkg.com/rst-selector-parser/-/rst-selector-parser-2.2.3.tgz", + "resolved": "https://registry.npmjs.org/rst-selector-parser/-/rst-selector-parser-2.2.3.tgz", "integrity": "sha1-gbIw6i/MYGbInjRy3nlChdmwPZE=", "dev": true, "requires": { From 7db222664d806c2524fbf3ee7aa13948d578b87d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 09:56:16 +0100 Subject: [PATCH 03/12] Fixed tests --- package-lock.json | 14 +++---- test/App.test.js | 5 ++- test/servers/helpers/ServerError.test.js | 42 +++++++++++++------- test/servers/reducers/selectedServer.test.js | 2 +- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7945df60..175f0e87 100644 --- a/package-lock.json +++ b/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": { diff --git a/test/App.test.js b/test/App.test.js index 428f3b1d..15208214 100644 --- a/test/App.test.js +++ b/test/App.test.js @@ -20,13 +20,14 @@ describe('', () => { 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); }); diff --git a/test/servers/helpers/ServerError.test.js b/test/servers/helpers/ServerError.test.js index 1e9d7417..32e1871c 100644 --- a/test/servers/helpers/ServerError.test.js +++ b/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('', () => { 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(); + ])('renders expected information for type "%s"', (type, textsToFind) => { + wrapper = shallow( + + + + ); 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); + } }); }); }); diff --git a/test/servers/reducers/selectedServer.test.js b/test/servers/reducers/selectedServer.test.js index 4cecf8cd..7c6ab7d0 100644 --- a/test/servers/reducers/selectedServer.test.js +++ b/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({}); From f6baedc6557158f99f47d1a7022b60f0976f8ab4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 10:33:23 +0100 Subject: [PATCH 04/12] Converted CreateServer into functional component --- src/servers/CreateServer.js | 86 ++++++++++++------------- src/servers/services/provideServices.js | 2 +- test/servers/CreateServer.test.js | 35 ++++------ 3 files changed, 55 insertions(+), 68 deletions(-) diff --git a/src/servers/CreateServer.js b/src/servers/CreateServer.js index 42f41410..8ce3fd02 100644 --- a/src/servers/CreateServer.js +++ b/src/servers/CreateServer.js @@ -1,46 +1,34 @@ -import { assoc, dissoc, pipe } from 'ramda'; -import React from 'react'; +import React, { useState, useEffect } from 'react'; import { v4 as uuid } from 'uuid'; import PropTypes from 'prop-types'; import './CreateServer.scss'; 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, resetSelectedServer }) => { + const [ name, setName ] = useState(''); + const [ url, setUrl ] = useState(''); + const [ apiKey, setApiKey ] = useState(''); + const [ serversImported, setServersImported ] = useStateFlagTimeout(false, SHOW_IMPORT_MSG_TIME); + const { push } = history; + const handleSubmit = (e) => { + e.preventDefault(); - state = { - name: '', - url: '', - apiKey: '', - serversImported: false, - }; + const id = uuid(); + const server = { id, name, url, apiKey }; - 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') => ( + createServer(server); + push(`/server/${id}/list-short-urls/1`); + }; + const renderInputGroup = (id, placeholder, value, setState, type = 'text') => (
); + useEffect(() => { + resetSelectedServer(); // FIXME Only when no serverId exists + }, []); + return (
-
- {renderInputGroup('name', 'Name')} - {renderInputGroup('url', 'URL', 'url')} - {renderInputGroup('apiKey', 'API key')} + + {renderInputGroup('name', 'Name', name, setName)} + {renderInputGroup('url', 'URL', url, setUrl, 'url')} + {renderInputGroup('apiKey', 'API key', apiKey, setApiKey)}
- stateFlagTimeout(this.setState.bind(this), 'serversImported', true, SHOW_IMPORT_MSG_TIME)} - /> +
- {this.state.serversImported && ( + {serversImported && (
@@ -85,7 +75,11 @@ const CreateServer = (ImportServersBtn, stateFlagTimeout) => class CreateServer
); - } + }; + + CreateServerComp.propTypes = propTypes; + + return CreateServerComp; }; export default CreateServer; diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index 5533372f..54c25130 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -14,7 +14,7 @@ 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('ServersDropdown', ServersDropdown, 'ServersExporter'); diff --git a/test/servers/CreateServer.test.js b/test/servers/CreateServer.test.js index 69a7888e..8523b200 100644 --- a/test/servers/CreateServer.test.js +++ b/test/servers/CreateServer.test.js @@ -10,19 +10,24 @@ describe('', () => { const historyMock = { push: jest.fn(), }; - - beforeEach(() => { - createServerMock.mockReset(); - - const CreateServer = createServerConstruct(ImportServersBtn); + const createWrapper = (serversImported = false) => { + const CreateServer = createServerConstruct(ImportServersBtn, () => [ serversImported, () => '' ]); wrapper = shallow( ); + + return wrapper; + }; + + afterEach(() => { + jest.resetAllMocks(); + wrapper && wrapper.unmount(); }); - afterEach(() => wrapper.unmount()); it('renders components', () => { + const wrapper = createWrapper(); + expect(wrapper.find('#name')).toHaveLength(1); expect(wrapper.find('#url')).toHaveLength(1); expect(wrapper.find('#apiKey')).toHaveLength(1); @@ -31,11 +36,13 @@ describe('', () => { }); 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 wrapper = createWrapper(); const form = wrapper.find('form'); form.simulate('submit', { preventDefault() { @@ -45,18 +52,4 @@ describe('', () => { 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'); - }); }); From 0aebaa4da193e7c27140f8c357406548bace40a5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 10:49:59 +0100 Subject: [PATCH 05/12] Extracted logic to render horizontal form groups to their own components --- src/servers/CreateServer.js | 30 ++++++----------------------- src/utils/HorizontalFormGroup.js | 32 +++++++++++++++++++++++++++++++ test/servers/CreateServer.test.js | 5 ++--- 3 files changed, 40 insertions(+), 27 deletions(-) create mode 100644 src/utils/HorizontalFormGroup.js diff --git a/src/servers/CreateServer.js b/src/servers/CreateServer.js index 8ce3fd02..81375eaa 100644 --- a/src/servers/CreateServer.js +++ b/src/servers/CreateServer.js @@ -1,6 +1,7 @@ import React, { useState, useEffect } from 'react'; import { v4 as uuid } from 'uuid'; import PropTypes from 'prop-types'; +import { HorizontalFormGroup } from '../utils/HorizontalFormGroup'; import './CreateServer.scss'; const SHOW_IMPORT_MSG_TIME = 4000; @@ -13,12 +14,11 @@ const propTypes = { }; const CreateServer = (ImportServersBtn, useStateFlagTimeout) => { - const CreateServerComp = ({ createServer, history, resetSelectedServer }) => { + const CreateServerComp = ({ createServer, history: { push }, resetSelectedServer }) => { const [ name, setName ] = useState(''); const [ url, setUrl ] = useState(''); const [ apiKey, setApiKey ] = useState(''); const [ serversImported, setServersImported ] = useStateFlagTimeout(false, SHOW_IMPORT_MSG_TIME); - const { push } = history; const handleSubmit = (e) => { e.preventDefault(); @@ -28,35 +28,17 @@ const CreateServer = (ImportServersBtn, useStateFlagTimeout) => { createServer(server); push(`/server/${id}/list-short-urls/1`); }; - const renderInputGroup = (id, placeholder, value, setState, type = 'text') => ( -
- -
- setState(e.target.value)} - /> -
-
- ); useEffect(() => { - resetSelectedServer(); // FIXME Only when no serverId exists + resetSelectedServer(); }, []); return (
- {renderInputGroup('name', 'Name', name, setName)} - {renderInputGroup('url', 'URL', url, setUrl, 'url')} - {renderInputGroup('apiKey', 'API key', apiKey, setApiKey)} + Name + URL + API key
diff --git a/src/utils/HorizontalFormGroup.js b/src/utils/HorizontalFormGroup.js new file mode 100644 index 00000000..2586c9c3 --- /dev/null +++ b/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 }) => ( +
+ +
+ onChange(e.target.value)} + /> +
+
+); + +HorizontalFormGroup.propTypes = propTypes; diff --git a/test/servers/CreateServer.test.js b/test/servers/CreateServer.test.js index 8523b200..b3e5bc1c 100644 --- a/test/servers/CreateServer.test.js +++ b/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 { HorizontalFormGroup } from '../../src/utils/HorizontalFormGroup'; describe('', () => { let wrapper; @@ -28,9 +29,7 @@ describe('', () => { it('renders components', () => { const wrapper = createWrapper(); - expect(wrapper.find('#name')).toHaveLength(1); - expect(wrapper.find('#url')).toHaveLength(1); - expect(wrapper.find('#apiKey')).toHaveLength(1); + expect(wrapper.find(HorizontalFormGroup)).toHaveLength(3); expect(wrapper.find(ImportServersBtn)).toHaveLength(1); expect(wrapper.find('.create-server__import-success-msg')).toHaveLength(0); }); From fb0ebddf280fa65e2d46ea9ae8eefaa0f3efa6f1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 11:29:20 +0100 Subject: [PATCH 06/12] Created component to edit existing servers --- src/App.js | 4 +- src/container/index.js | 2 +- src/servers/EditServer.js | 65 ++++++++++++++++++++ src/servers/reducers/server.js | 2 + src/servers/services/ServersService.js | 10 +++ src/servers/services/provideServices.js | 7 ++- test/App.test.js | 2 +- test/servers/reducers/server.test.js | 27 +++++++- test/servers/services/ServersService.test.js | 56 +++++++++++------ 9 files changed, 149 insertions(+), 26 deletions(-) create mode 100644 src/servers/EditServer.js diff --git a/src/App.js b/src/App.js index 5547e1e3..4cecebff 100644 --- a/src/App.js +++ b/src/App.js @@ -3,7 +3,7 @@ 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) => () => (
@@ -11,7 +11,7 @@ const App = (MainHeader, Home, MenuLayout, CreateServer) => () => ( - + diff --git a/src/container/index.js b/src/container/index.js index 483c8acd..771a46e8 100644 --- a/src/container/index.js +++ b/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); diff --git a/src/servers/EditServer.js b/src/servers/EditServer.js new file mode 100644 index 00000000..f3adbc58 --- /dev/null +++ b/src/servers/EditServer.js @@ -0,0 +1,65 @@ +import React, { useState, useEffect } from 'react'; +import PropTypes, { serverType } from 'prop-types'; +import { HorizontalFormGroup } from '../utils/HorizontalFormGroup'; +import './CreateServer.scss'; +import Message from '../utils/Message'; + +const propTypes = { + editServer: PropTypes.func, + selectServer: PropTypes.func, + selectedServer: serverType, + match: PropTypes.object, + history: PropTypes.shape({ + push: PropTypes.func, + }), +}; + +export const EditServer = (ServerError) => { + const EditServerComp = ({ editServer, selectServer, selectedServer, match, history: { push } }) => { + const [ name, setName ] = useState(''); + const [ url, setUrl ] = useState(''); + const [ apiKey, setApiKey ] = useState(''); + const { params: { serverId } } = match; + const handleSubmit = (e) => { + e.preventDefault(); + + editServer(serverId, { name, url, apiKey }); + push(`/server/${serverId}/list-short-urls/1`); + }; + + useEffect(() => { + selectServer(serverId); + }, [ serverId ]); + useEffect(() => { + selectedServer && setName(selectedServer.name); + selectedServer && setUrl(selectedServer.url); + selectedServer && setApiKey(selectedServer.apiKey); + }, [ selectedServer ]); + + if (!selectedServer) { + return ; + } + + if (selectedServer.serverNotFound) { + return ; + } + + return ( +
+ + Name + URL + API key + +
+ +
+ +
+ ); + }; + + EditServerComp.propTypes = propTypes; + + return EditServerComp; +}; diff --git a/src/servers/reducers/server.js b/src/servers/reducers/server.js index 576eca14..e665751c 100644 --- a/src/servers/reducers/server.js +++ b/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( diff --git a/src/servers/services/ServersService.js b/src/servers/services/ServersService.js index e8ffdb68..9ec44afa 100644 --- a/src/servers/services/ServersService.js +++ b/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)); + } } diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index 54c25130..0516f665 100644 --- a/src/servers/services/provideServices.js +++ b/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'; @@ -17,6 +18,9 @@ const provideServices = (bottle, connect, withRouter) => { 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' ])); @@ -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); diff --git a/test/App.test.js b/test/App.test.js index 15208214..668239ca 100644 --- a/test/App.test.js +++ b/test/App.test.js @@ -9,7 +9,7 @@ describe('', () => { const MainHeader = () => ''; beforeEach(() => { - const App = appFactory(MainHeader, identity, identity, identity); + const App = appFactory(MainHeader, identity, identity, identity, identity); wrapper = shallow(); }); diff --git a/test/servers/reducers/server.test.js b/test/servers/reducers/server.test.js index f819ee34..f6c55325 100644 --- a/test/servers/reducers/server.test.js +++ b/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(); }); }); }); diff --git a/test/servers/services/ServersService.test.js b/test/servers/services/ServersService.test.js index 87346c82..3c8b805f 100644 --- a/test/servers/services/ServersService.test.js +++ b/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' }, + }); + }); + }); }); From 6d44ac1e0c6bb1f0fb77457b50cd22b098beac57 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 11:59:07 +0100 Subject: [PATCH 07/12] Created common component that can be used both for create and edit servers --- src/servers/CreateServer.js | 41 +++++++++---------------- src/servers/helpers/ServerForm.js | 41 +++++++++++++++++++++++++ test/servers/CreateServer.test.js | 11 +++---- test/servers/helpers/ServerForm.test.js | 33 ++++++++++++++++++++ 4 files changed, 93 insertions(+), 33 deletions(-) create mode 100644 src/servers/helpers/ServerForm.js create mode 100644 test/servers/helpers/ServerForm.test.js diff --git a/src/servers/CreateServer.js b/src/servers/CreateServer.js index 81375eaa..235ecd82 100644 --- a/src/servers/CreateServer.js +++ b/src/servers/CreateServer.js @@ -1,8 +1,8 @@ -import React, { useState, useEffect } from 'react'; +import React, { useEffect } from 'react'; import { v4 as uuid } from 'uuid'; import PropTypes from 'prop-types'; -import { HorizontalFormGroup } from '../utils/HorizontalFormGroup'; import './CreateServer.scss'; +import { ServerForm } from './helpers/ServerForm'; const SHOW_IMPORT_MSG_TIME = 4000; const propTypes = { @@ -15,15 +15,10 @@ const propTypes = { const CreateServer = (ImportServersBtn, useStateFlagTimeout) => { const CreateServerComp = ({ createServer, history: { push }, resetSelectedServer }) => { - const [ name, setName ] = useState(''); - const [ url, setUrl ] = useState(''); - const [ apiKey, setApiKey ] = useState(''); const [ serversImported, setServersImported ] = useStateFlagTimeout(false, SHOW_IMPORT_MSG_TIME); - const handleSubmit = (e) => { - e.preventDefault(); - + const handleSubmit = (serverData) => { const id = uuid(); - const server = { id, name, url, apiKey }; + const server = { id, ...serverData }; createServer(server); push(`/server/${id}/list-short-urls/1`); @@ -35,26 +30,20 @@ const CreateServer = (ImportServersBtn, useStateFlagTimeout) => { return (
-
- Name - URL - API key + + + + -
- - -
- - {serversImported && ( -
-
-
- Servers properly imported. You can now select one from the list :) -
+ {serversImported && ( +
+
+
+ Servers properly imported. You can now select one from the list :)
- )} - +
+ )}
); }; diff --git a/src/servers/helpers/ServerForm.js b/src/servers/helpers/ServerForm.js new file mode 100644 index 00000000..57650f09 --- /dev/null +++ b/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 ( +
+ Name + URL + API key + +
{children}
+
+ ); +}; + +ServerForm.propTypes = propTypes; diff --git a/test/servers/CreateServer.test.js b/test/servers/CreateServer.test.js index b3e5bc1c..cab51a60 100644 --- a/test/servers/CreateServer.test.js +++ b/test/servers/CreateServer.test.js @@ -2,7 +2,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { identity } from 'ramda'; import createServerConstruct from '../../src/servers/CreateServer'; -import { HorizontalFormGroup } from '../../src/utils/HorizontalFormGroup'; +import { ServerForm } from '../../src/servers/helpers/ServerForm'; describe('', () => { let wrapper; @@ -29,8 +29,7 @@ describe('', () => { it('renders components', () => { const wrapper = createWrapper(); - expect(wrapper.find(HorizontalFormGroup)).toHaveLength(3); - expect(wrapper.find(ImportServersBtn)).toHaveLength(1); + expect(wrapper.find(ServerForm)).toHaveLength(1); expect(wrapper.find('.create-server__import-success-msg')).toHaveLength(0); }); @@ -42,11 +41,9 @@ describe('', () => { it('creates server and redirects to it when form is submitted', () => { const wrapper = createWrapper(); - const form = wrapper.find('form'); + const form = wrapper.find(ServerForm); - form.simulate('submit', { preventDefault() { - return ''; - } }); + form.simulate('submit', {}); expect(createServerMock).toHaveBeenCalledTimes(1); expect(historyMock.push).toHaveBeenCalledTimes(1); diff --git a/test/servers/helpers/ServerForm.test.js b/test/servers/helpers/ServerForm.test.js new file mode 100644 index 00000000..855e8c1e --- /dev/null +++ b/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('', () => { + let wrapper; + const onSubmit = jest.fn(); + + beforeEach(() => { + wrapper = shallow(Something); + }); + + 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(); + }); +}); From 5d8af1a0e57fbda2dc4c7aa10d5393ad5ad1de6c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 12:02:19 +0100 Subject: [PATCH 08/12] Simplified EditServer component by wrapping ServerForm --- src/servers/EditServer.js | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/src/servers/EditServer.js b/src/servers/EditServer.js index f3adbc58..19fab6f4 100644 --- a/src/servers/EditServer.js +++ b/src/servers/EditServer.js @@ -1,8 +1,8 @@ -import React, { useState, useEffect } from 'react'; +import React, { useEffect } from 'react'; import PropTypes, { serverType } from 'prop-types'; -import { HorizontalFormGroup } from '../utils/HorizontalFormGroup'; import './CreateServer.scss'; import Message from '../utils/Message'; +import { ServerForm } from './helpers/ServerForm'; const propTypes = { editServer: PropTypes.func, @@ -16,25 +16,15 @@ const propTypes = { export const EditServer = (ServerError) => { const EditServerComp = ({ editServer, selectServer, selectedServer, match, history: { push } }) => { - const [ name, setName ] = useState(''); - const [ url, setUrl ] = useState(''); - const [ apiKey, setApiKey ] = useState(''); const { params: { serverId } } = match; - const handleSubmit = (e) => { - e.preventDefault(); - - editServer(serverId, { name, url, apiKey }); + const handleSubmit = (serverData) => { + editServer(serverId, serverData); push(`/server/${serverId}/list-short-urls/1`); }; useEffect(() => { selectServer(serverId); }, [ serverId ]); - useEffect(() => { - selectedServer && setName(selectedServer.name); - selectedServer && setUrl(selectedServer.url); - selectedServer && setApiKey(selectedServer.apiKey); - }, [ selectedServer ]); if (!selectedServer) { return ; @@ -46,15 +36,9 @@ export const EditServer = (ServerError) => { return (
-
- Name - URL - API key - -
- -
-
+ + +
); }; From 24f2deda466b1bdf61a2cc65f52083841ac080dd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 13:43:12 +0100 Subject: [PATCH 09/12] Moved common code to handle currently selected server to HOC --- src/common/MenuLayout.js | 18 ++------ src/servers/EditServer.js | 31 ++++---------- src/servers/helpers/withSelectedServer.js | 35 +++++++++++++++ test/servers/EditServer.test.js | 52 +++++++++++++++++++++++ 4 files changed, 98 insertions(+), 38 deletions(-) create mode 100644 src/servers/helpers/withSelectedServer.js create mode 100644 test/servers/EditServer.test.js diff --git a/src/common/MenuLayout.js b/src/common/MenuLayout.js index 1951a63f..ad63d427 100644 --- a/src/common/MenuLayout.js +++ b/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 ; - } - - if (selectedServer.serverNotFound) { - return ; - } - if (selectedServer.serverNotReachable) { return ; } @@ -91,7 +79,7 @@ const MenuLayout = (TagsList, ShortUrls, AsideMenu, CreateShortUrl, ShortUrlVisi MenuLayoutComp.propTypes = propTypes; - return MenuLayoutComp; + return withSelectedServer(MenuLayoutComp, ServerError); }; export default MenuLayout; diff --git a/src/servers/EditServer.js b/src/servers/EditServer.js index 19fab6f4..c4d4b0eb 100644 --- a/src/servers/EditServer.js +++ b/src/servers/EditServer.js @@ -1,39 +1,24 @@ -import React, { useEffect } from 'react'; -import PropTypes, { serverType } from 'prop-types'; -import './CreateServer.scss'; -import Message from '../utils/Message'; +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, - selectServer: PropTypes.func, selectedServer: serverType, - match: PropTypes.object, history: PropTypes.shape({ push: PropTypes.func, }), }; export const EditServer = (ServerError) => { - const EditServerComp = ({ editServer, selectServer, selectedServer, match, history: { push } }) => { - const { params: { serverId } } = match; + const EditServerComp = ({ editServer, selectedServer, history: { push } }) => { const handleSubmit = (serverData) => { - editServer(serverId, serverData); - push(`/server/${serverId}/list-short-urls/1`); + editServer(selectedServer.id, serverData); + push(`/server/${selectedServer.id}/list-short-urls/1`); }; - useEffect(() => { - selectServer(serverId); - }, [ serverId ]); - - if (!selectedServer) { - return ; - } - - if (selectedServer.serverNotFound) { - return ; - } - return (
@@ -45,5 +30,5 @@ export const EditServer = (ServerError) => { EditServerComp.propTypes = propTypes; - return EditServerComp; + return withSelectedServer(EditServerComp, ServerError); }; diff --git a/src/servers/helpers/withSelectedServer.js b/src/servers/helpers/withSelectedServer.js new file mode 100644 index 00000000..3c524e61 --- /dev/null +++ b/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 ; + } + + if (selectedServer.serverNotFound) { + return ; + } + + return ; + }; + + Component.propTypes = propTypes; + + return Component; +}; diff --git a/test/servers/EditServer.test.js b/test/servers/EditServer.test.js new file mode 100644 index 00000000..36d81d2c --- /dev/null +++ b/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('', () => { + 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( + + ); + }); + + 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); + }); +}); From 35a62f1fb191cc71fe446b62b41a677c432552e5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 14:03:41 +0100 Subject: [PATCH 10/12] Added link to edit existing servers --- src/common/AsideMenu.js | 22 ++++++++++++++++++---- src/common/AsideMenu.scss | 3 +++ test/common/AsideMenu.test.js | 2 +- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/common/AsideMenu.js b/src/common/AsideMenu.js index 433163ba..dca91132 100644 --- a/src/common/AsideMenu.js +++ b/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 }) => ( - +const AsideMenuItem = ({ children, to, className, ...rest }) => ( + {children} ); @@ -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,7 +59,10 @@ const AsideMenu = (DeleteServerButton) => { Manage tags - + + + Edit this server + ', () => { 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')); }); From e4f7ded8e2c335353c9c5f0ab7063ec094e7139c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 14:04:33 +0100 Subject: [PATCH 11/12] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd68d57c..0de9d24d 100644 --- a/CHANGELOG.md +++ b/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 From 77b91811500396f4a18be22bf19857be793329cc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Mar 2020 14:23:57 +0100 Subject: [PATCH 12/12] Replaced hardcoded color by sass var --- src/servers/helpers/ServerError.scss | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/servers/helpers/ServerError.scss b/src/servers/helpers/ServerError.scss index a4d991b9..5d093b62 100644 --- a/src/servers/helpers/ServerError.scss +++ b/src/servers/helpers/ServerError.scss @@ -1,3 +1,5 @@ +@import '../../utils/base'; + .server-error__container { text-align: center; display: flex; @@ -6,7 +8,7 @@ } .server-error__delete-btn { - color: #dc3545; + color: $dangerColor; cursor: pointer; }