From 1b336e2e80f9a0732fe751dcba77e47a113d46eb Mon Sep 17 00:00:00 2001
From: Matthias Kretschmann <m@kretschmann.io>
Date: Tue, 11 Jun 2019 20:27:02 +0200
Subject: [PATCH 1/3] version number fixes

* add loading state
* add error state
* handle connection errors
* backwards compatibility for all components
* small spinner variant
---
 .../src/components/atoms/Spinner.module.scss  |  14 ++
 client/src/components/atoms/Spinner.tsx       |  32 +++--
 .../VersionNumbers/VersionTable.module.scss   |   5 +
 .../atoms/VersionNumbers/VersionTable.tsx     |  58 +++++---
 .../components/atoms/VersionNumbers/index.tsx | 131 ++++++++++++------
 library.json                                  |   4 +-
 6 files changed, 171 insertions(+), 73 deletions(-)

diff --git a/client/src/components/atoms/Spinner.module.scss b/client/src/components/atoms/Spinner.module.scss
index 2fd9507..22e4117 100644
--- a/client/src/components/atoms/Spinner.module.scss
+++ b/client/src/components/atoms/Spinner.module.scss
@@ -29,6 +29,20 @@
     padding-top: $spacer / 4;
 }
 
+.small {
+    composes: spinner;
+    margin: 0;
+    display: inline-block;
+
+    &:before {
+        width: $font-size-small;
+        height: $font-size-small;
+        margin-top: -($font-size-small);
+        margin-left: -($font-size-small / 2);
+        border-width: .1rem;
+    }
+}
+
 @keyframes spinner {
     to {
         transform: rotate(360deg);
diff --git a/client/src/components/atoms/Spinner.tsx b/client/src/components/atoms/Spinner.tsx
index 0759275..b8013f3 100644
--- a/client/src/components/atoms/Spinner.tsx
+++ b/client/src/components/atoms/Spinner.tsx
@@ -1,15 +1,27 @@
 import React from 'react'
 import styles from './Spinner.module.scss'
 
-const Spinner = ({ message }: { message?: string }) => (
-    <div className={styles.spinner}>
-        {message && (
-            <div
-                className={styles.spinnerMessage}
-                dangerouslySetInnerHTML={{ __html: message }}
-            />
-        )}
-    </div>
-)
+const Spinner = ({
+    message,
+    small,
+    className
+}: {
+    message?: string
+    small?: boolean
+    className?: string
+}) => {
+    const classes = className || (small ? styles.small : styles.spinner)
+
+    return (
+        <div className={classes}>
+            {message && (
+                <div
+                    className={styles.spinnerMessage}
+                    dangerouslySetInnerHTML={{ __html: message }}
+                />
+            )}
+        </div>
+    )
+}
 
 export default Spinner
diff --git a/client/src/components/atoms/VersionNumbers/VersionTable.module.scss b/client/src/components/atoms/VersionNumbers/VersionTable.module.scss
index 23e31f2..06f9b2e 100644
--- a/client/src/components/atoms/VersionNumbers/VersionTable.module.scss
+++ b/client/src/components/atoms/VersionNumbers/VersionTable.module.scss
@@ -52,3 +52,8 @@
         }
     }
 }
+
+.spinner {
+    composes: spinner, small from '../Spinner.module.scss';
+    margin-right: $spacer;
+}
diff --git a/client/src/components/atoms/VersionNumbers/VersionTable.tsx b/client/src/components/atoms/VersionNumbers/VersionTable.tsx
index 4533fa0..fcab197 100644
--- a/client/src/components/atoms/VersionNumbers/VersionTable.tsx
+++ b/client/src/components/atoms/VersionNumbers/VersionTable.tsx
@@ -2,6 +2,7 @@ import React, { Fragment } from 'react'
 import { VersionNumbersState as VersionTableProps } from '.'
 import styles from './VersionTable.module.scss'
 import slugify from '@sindresorhus/slugify'
+import Spinner from '../Spinner'
 
 const VersionTableContracts = ({
     contracts,
@@ -12,23 +13,24 @@ const VersionTableContracts = ({
 }) => (
     <table>
         <tbody>
-            {Object.keys(contracts).map(key => (
-                <tr key={key}>
-                    <td>
-                        <span className={styles.label}>{key}</span>
-                    </td>
-                    <td>
-                        <a
-                            href={`https://submarine${network === 'duero' &&
-                                '.duero'}.dev-ocean.com/address/${
-                                contracts[key]
-                            }`}
-                        >
-                            <code>{contracts[key]}</code>
-                        </a>
-                    </td>
-                </tr>
-            ))}
+            {contracts &&
+                Object.keys(contracts).map(key => (
+                    <tr key={key}>
+                        <td>
+                            <span className={styles.label}>{key}</span>
+                        </td>
+                        <td>
+                            <a
+                                href={`https://submarine${network === 'duero' &&
+                                    '.duero'}.dev-ocean.com/address/${
+                                    contracts[key]
+                                }`}
+                            >
+                                <code>{contracts[key]}</code>
+                            </a>
+                        </td>
+                    </tr>
+                ))}
         </tbody>
     </table>
 )
@@ -42,18 +44,30 @@ const VersionTable = ({ data }: { data: VersionTableProps }) => (
                         <tr key={key}>
                             <td>
                                 <a
-                                    href={`https://github.com/oceanprotocol/${slugify(
-                                        value.software
-                                    )}`}
+                                    href={
+                                        value.software &&
+                                        `https://github.com/oceanprotocol/${slugify(
+                                            value.software
+                                        )}`
+                                    }
                                 >
                                     <strong>{value.software}</strong>
                                 </a>
                             </td>
                             <td>
-                                <code>v{value.version}</code>
+                                {value.isLoading ? (
+                                    <Spinner small className={styles.spinner} />
+                                ) : value.version ? (
+                                    <>
+                                        <code>v{value.version}</code>
+                                        {value.network && `(${value.network})`}
+                                    </>
+                                ) : (
+                                    'Could not get version'
+                                )}
                             </td>
                         </tr>
-                        {key === 'keeperContracts' && (
+                        {key === 'keeperContracts' && data.brizo.contracts && (
                             <tr>
                                 <td colSpan={2}>
                                     <VersionTableContracts
diff --git a/client/src/components/atoms/VersionNumbers/index.tsx b/client/src/components/atoms/VersionNumbers/index.tsx
index f706942..e39e0bf 100644
--- a/client/src/components/atoms/VersionNumbers/index.tsx
+++ b/client/src/components/atoms/VersionNumbers/index.tsx
@@ -36,10 +36,12 @@ export interface VersionNumbersState {
         version: string
     }
     aquarius: {
+        isLoading: boolean
         software: string
         version: string
     }
     brizo: {
+        isLoading: boolean
         software: string
         version: string
         network: string
@@ -48,13 +50,16 @@ export interface VersionNumbersState {
         contracts: any
     }
     keeperContracts: {
+        isLoading: boolean
         software: string
         version: string
+        network: string
         contracts: any
     }
     faucet: {
-        software: string
-        version: string
+        isLoading: boolean
+        software?: string
+        version?: string
     }
 }
 
@@ -68,73 +73,121 @@ export default class VersionNumbers extends PureComponent<
             software: 'Squid-js',
             version: versionSquid
         },
-        aquarius: { software: 'Aquarius', version: '0.0.0' },
+        aquarius: {
+            isLoading: true,
+            software: 'Aquarius',
+            version: ''
+        },
         brizo: {
+            isLoading: true,
             software: 'Brizo',
-            version: '0.0.0',
+            version: '',
             contracts: {} as any,
             network: '',
             'keeper-version': '0.0.0',
             'keeper-url': ''
         },
         keeperContracts: {
+            isLoading: true,
             software: 'Keeper Contracts',
-            version: '0.0.0',
+            version: '',
             contracts: {} as any,
             network: ''
         },
-        faucet: { software: 'Faucet', version: '0.0.0' }
+        faucet: {
+            isLoading: true,
+            software: 'Faucet',
+            version: ''
+        }
     }
 
     // for canceling axios requests
     public signal = axios.CancelToken.source()
 
     public componentWillMount() {
-        this.setComponentVersions()
+        this.setAquarius()
+        this.setBrizoAndKeeper()
+        this.setFaucet()
     }
 
     public componentWillUnmount() {
         this.signal.cancel()
     }
 
-    private async setComponentVersions() {
-        try {
-            const aquarius = await this.getData(
-                aquariusScheme,
-                aquariusHost,
-                aquariusPort
-            )
-            aquarius.version !== undefined && this.setState({ aquarius })
+    private async setAquarius() {
+        const aquarius = await this.getData(
+            aquariusScheme,
+            aquariusHost,
+            aquariusPort
+        )
+        aquarius.version !== undefined &&
+            this.setState({ aquarius: { isLoading: false, ...aquarius } })
+    }
 
-            const brizo = await this.getData(brizoScheme, brizoHost, brizoPort)
-            brizo.version !== undefined &&
-                this.setState({
-                    brizo,
-                    keeperContracts: {
-                        ...this.state.keeperContracts,
-                        version: brizo['keeper-version'].replace('v', ''),
-                        contracts: brizo.contracts
-                    }
-                })
+    private async setBrizoAndKeeper() {
+        const brizo = await this.getData(brizoScheme, brizoHost, brizoPort)
 
-            const faucet = await this.getData(
-                faucetScheme,
-                faucetHost,
-                faucetPort
-            )
+        const keeperVersion =
+            brizo['keeper-version'] && brizo['keeper-version'].replace('v', '')
+        const keeperNetwork =
+            brizo['keeper-url'] &&
+            new URL(brizo['keeper-url']).hostname.split('.')[0]
 
-            faucet.version !== undefined && this.setState({ faucet })
-        } catch (error) {
-            !axios.isCancel(error) && Logger.error(error.message)
+        brizo.version !== undefined &&
+            this.setState({
+                brizo: {
+                    isLoading: false,
+                    ...brizo
+                },
+                keeperContracts: {
+                    ...this.state.keeperContracts,
+                    isLoading: false,
+                    version: keeperVersion,
+                    contracts: brizo.contracts,
+                    network: keeperNetwork
+                }
+            })
+    }
+
+    private async setFaucet() {
+        const faucet = await this.getData(faucetScheme, faucetHost, faucetPort)
+
+        // backwards compatibility
+        function IsJsonString(str: string) {
+            try {
+                JSON.parse(str)
+            } catch (e) {
+                return false
+            }
+            return true
         }
+
+        IsJsonString(faucet) === false &&
+            this.setState({
+                faucet: { ...this.state.faucet, isLoading: false }
+            })
+
+        // the new thing
+        faucet.version !== undefined &&
+            this.setState({ faucet: { isLoading: false, ...faucet } })
     }
 
     private async getData(scheme: string, host: string, port: number | string) {
-        const { data } = await axios.get(`${scheme}://${host}:${port}`, {
-            headers: { Accept: 'application/json' },
-            cancelToken: this.signal.token
-        })
-        return data
+        try {
+            const response = await axios.get(`${scheme}://${host}:${port}`, {
+                headers: { Accept: 'application/json' },
+                cancelToken: this.signal.token
+            })
+
+            if (response.status !== 200) {
+                Logger.error(response.statusText)
+                return
+            }
+
+            return response.data
+        } catch (error) {
+            !axios.isCancel(error) && Logger.error(error.message)
+        }
     }
 
     public render() {
@@ -152,7 +205,7 @@ export default class VersionNumbers extends PureComponent<
         return minimal ? (
             <p className={styles.versionsMinimal}>
                 <a title={mimimalOutput} href={'/about'}>
-                    v{commons.version} ({brizo.network})
+                    v{commons.version} {brizo.network && `(${brizo.network})`}
                 </a>
             </p>
         ) : (
diff --git a/library.json b/library.json
index 1eadf34..653a471 100644
--- a/library.json
+++ b/library.json
@@ -11,7 +11,7 @@
     },
     {
       "name": "brizo",
-      "version": "~0.3.6"
+      "version": "~0.3.9"
     },
     {
       "name": "aquarius",
@@ -23,7 +23,7 @@
     },
     {
       "name": "faucet",
-      "version": "~0.2.0"
+      "version": "~0.2.4"
     }
   ]
 }

From 264a066874dbe55950f54754924e2a5c4d2b69ec Mon Sep 17 00:00:00 2001
From: Matthias Kretschmann <m@kretschmann.io>
Date: Tue, 11 Jun 2019 22:15:14 +0200
Subject: [PATCH 2/3] update tests

---
 .../atoms/VersionNumbers/index.test.tsx       | 101 +++++++++++++++++-
 .../components/atoms/VersionNumbers/index.tsx |  29 ++---
 .../atoms/VersionNumbers/utils.test.ts        |  11 ++
 .../components/atoms/VersionNumbers/utils.ts  |   8 ++
 4 files changed, 128 insertions(+), 21 deletions(-)
 create mode 100644 client/src/components/atoms/VersionNumbers/utils.test.ts
 create mode 100644 client/src/components/atoms/VersionNumbers/utils.ts

diff --git a/client/src/components/atoms/VersionNumbers/index.test.tsx b/client/src/components/atoms/VersionNumbers/index.test.tsx
index 330b3f9..94ec2c7 100644
--- a/client/src/components/atoms/VersionNumbers/index.test.tsx
+++ b/client/src/components/atoms/VersionNumbers/index.test.tsx
@@ -1,29 +1,124 @@
 import React from 'react'
 import { render, waitForElement } from '@testing-library/react'
 import mockAxios from 'jest-mock-axios'
-import VersionNumbers from '.'
+import { StateMock } from '@react-mock/state'
+import { version as versionSquid } from '@oceanprotocol/squid/package.json'
+import VersionNumbers, { commonsVersion } from '.'
 
 afterEach(() => {
     mockAxios.reset()
 })
 
+const stateMock = {
+    commons: { software: 'Commons', version: commonsVersion },
+    squidJs: {
+        software: 'Squid-js',
+        version: versionSquid
+    },
+    aquarius: {
+        isLoading: false,
+        software: 'Aquarius',
+        version: ''
+    },
+    brizo: {
+        isLoading: false,
+        software: 'Brizo',
+        version: '',
+        contracts: {},
+        network: '',
+        'keeper-version': '0.0.0',
+        'keeper-url': ''
+    },
+    keeperContracts: {
+        isLoading: false,
+        software: 'Keeper Contracts',
+        version: '',
+        contracts: {},
+        network: ''
+    },
+    faucet: {
+        isLoading: false,
+        software: 'Faucet',
+        version: ''
+    }
+}
+
+const stateMockIncomplete = {
+    commons: { software: 'Commons', version: commonsVersion },
+    squidJs: {
+        software: 'Squid-js',
+        version: versionSquid
+    },
+    aquarius: {
+        isLoading: false,
+        software: 'Aquarius',
+        version: undefined
+    },
+    brizo: {
+        isLoading: false,
+        software: 'Brizo',
+        version: undefined,
+        contracts: undefined,
+        network: undefined,
+        'keeper-version': undefined,
+        'keeper-url': undefined
+    },
+    keeperContracts: {
+        isLoading: false,
+        software: 'Keeper Contracts',
+        version: undefined,
+        contracts: undefined,
+        network: undefined
+    },
+    faucet: {
+        isLoading: false,
+        software: 'Faucet',
+        version: undefined
+    }
+}
+
 const mockResponse = {
     data: {
         software: 'Brizo',
         version: '6.6.6',
         contracts: { Hello: 'Hello', Another: 'Hello' },
-        network: 'hello'
+        network: 'hello',
+        'keeper-url': 'https://squid.com',
+        'keeper-version': '6.6.6'
     }
 }
 
+const mockResponseFaulty = {
+    status: 404,
+    statusText: 'Not Found',
+    data: {}
+}
+
 describe('VersionNumbers', () => {
     it('renders without crashing', () => {
-        const { container } = render(<VersionNumbers />)
+        const { container } = render(
+            <StateMock state={stateMock}>
+                <VersionNumbers />
+            </StateMock>
+        )
         mockAxios.mockResponse(mockResponse)
         expect(mockAxios.get).toHaveBeenCalled()
         expect(container.firstChild).toBeInTheDocument()
     })
 
+    it('renders without proper component response', () => {
+        const { container } = render(
+            <StateMock state={stateMockIncomplete}>
+                <VersionNumbers />
+            </StateMock>
+        )
+        mockAxios.mockResponse(mockResponseFaulty)
+        expect(mockAxios.get).toHaveBeenCalled()
+        expect(container.querySelector('table')).toHaveTextContent(
+            'Could not get version'
+        )
+    })
+
     it('minimal component versions in link title, prefixed with `v`', async () => {
         const { getByTitle } = render(<VersionNumbers minimal />)
         mockAxios.mockResponse(mockResponse)
diff --git a/client/src/components/atoms/VersionNumbers/index.tsx b/client/src/components/atoms/VersionNumbers/index.tsx
index e39e0bf..e7edbfc 100644
--- a/client/src/components/atoms/VersionNumbers/index.tsx
+++ b/client/src/components/atoms/VersionNumbers/index.tsx
@@ -18,8 +18,9 @@ import {
 } from '../../../config'
 
 import VersionTable from './VersionTable'
+import { isJsonString } from './utils'
 
-const commonsVersion =
+export const commonsVersion =
     process.env.NODE_ENV === 'production' ? version : `${version}-dev`
 
 interface VersionNumbersProps {
@@ -120,7 +121,8 @@ export default class VersionNumbers extends PureComponent<
             aquariusHost,
             aquariusPort
         )
-        aquarius.version !== undefined &&
+        aquarius &&
+            aquarius.version !== undefined &&
             this.setState({ aquarius: { isLoading: false, ...aquarius } })
     }
 
@@ -133,7 +135,8 @@ export default class VersionNumbers extends PureComponent<
             brizo['keeper-url'] &&
             new URL(brizo['keeper-url']).hostname.split('.')[0]
 
-        brizo.version !== undefined &&
+        brizo &&
+            brizo.version !== undefined &&
             this.setState({
                 brizo: {
                     isLoading: false,
@@ -153,22 +156,14 @@ export default class VersionNumbers extends PureComponent<
         const faucet = await this.getData(faucetScheme, faucetHost, faucetPort)
 
         // backwards compatibility
-        function IsJsonString(str: string) {
-            try {
-                JSON.parse(str)
-            } catch (e) {
-                return false
-            }
-            return true
-        }
-
-        IsJsonString(faucet) === false &&
+        isJsonString(faucet) === false &&
             this.setState({
                 faucet: { ...this.state.faucet, isLoading: false }
             })
 
         // the new thing
-        faucet.version !== undefined &&
+        faucet &&
+            faucet.version !== undefined &&
             this.setState({ faucet: { isLoading: false, ...faucet } })
     }
 
@@ -179,10 +174,8 @@ export default class VersionNumbers extends PureComponent<
                 cancelToken: this.signal.token
             })
 
-            if (response.status !== 200) {
-                Logger.error(response.statusText)
-                return
-            }
+            // fail silently
+            if (response.status !== 200) return
 
             return response.data
         } catch (error) {
diff --git a/client/src/components/atoms/VersionNumbers/utils.test.ts b/client/src/components/atoms/VersionNumbers/utils.test.ts
new file mode 100644
index 0000000..0fa7677
--- /dev/null
+++ b/client/src/components/atoms/VersionNumbers/utils.test.ts
@@ -0,0 +1,11 @@
+import { isJsonString } from './utils'
+
+describe('isJsonString', () => {
+    it('detects json correctly', () => {
+        const testJson = isJsonString('{ "hello": "squid" }')
+        expect(testJson).toBeTruthy()
+
+        const testNonJson = isJsonString('<strong>')
+        expect(testNonJson).toBeFalsy()
+    })
+})
diff --git a/client/src/components/atoms/VersionNumbers/utils.ts b/client/src/components/atoms/VersionNumbers/utils.ts
new file mode 100644
index 0000000..0b12378
--- /dev/null
+++ b/client/src/components/atoms/VersionNumbers/utils.ts
@@ -0,0 +1,8 @@
+export function isJsonString(str: string) {
+    try {
+        JSON.parse(str)
+    } catch (e) {
+        return false
+    }
+    return true
+}

From 097cf8bd6393eb6ae7edf265bb2027eb9bfae6bd Mon Sep 17 00:00:00 2001
From: Matthias Kretschmann <m@kretschmann.io>
Date: Tue, 11 Jun 2019 22:32:54 +0200
Subject: [PATCH 3/3] ignore definition files for coverage reporting

---
 client/package.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/client/package.json b/client/package.json
index a1bb55f..7493d5c 100644
--- a/client/package.json
+++ b/client/package.json
@@ -73,7 +73,8 @@
   "jest": {
     "collectCoverageFrom": [
       "src/**/*.{ts,tsx}",
-      "!src/serviceWorker.ts"
+      "!src/serviceWorker.ts",
+      "!src/**/*.d.ts"
     ]
   }
 }