diff --git a/.dockerignore b/.dockerignore index 63f72310..1d878882 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,3 +5,4 @@ ./test ./shlink-web-client.gif ./dist +./docs diff --git a/CHANGELOG.md b/CHANGELOG.md index 916e231b..7e059687 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,24 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org). +## [3.4.1] - 2021-11-20 +### Added +* [#525](https://github.com/shlinkio/shlink-web-client/issues/525) Added docs section for Architectural Decision Records, including the one for servers "auto-connect". + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#520](https://github.com/shlinkio/shlink-web-client/issues/520) Fixed landing page scroll on mobile devices and improved its design. +* [#526](https://github.com/shlinkio/shlink-web-client/issues/526) Ensured exported servers do not include the `autoConnect` prop. + + ## [3.4.0] - 2021-11-11 ### Added * [#496](https://github.com/shlinkio/shlink-web-client/issues/496) Allowed to select "all visits" as the default interval for visits. diff --git a/docs/adr/2021-10-31-how-to-handle-setting-auto-connect-on-servers.md b/docs/adr/2021-10-31-how-to-handle-setting-auto-connect-on-servers.md new file mode 100644 index 00000000..2f1e8a6f --- /dev/null +++ b/docs/adr/2021-10-31-how-to-handle-setting-auto-connect-on-servers.md @@ -0,0 +1,51 @@ +# How to handle setting auto-connect on servers + +* Status: Accepted +* Date: 2021-10-31 + +## Context and problem statement + +A new feature has been requested, to allow auto-connecting to servers. The request specifically mentioned doing it automatically when there's only one server configured, but it can be extended a bit to allow setting an "auto-connect" server, regardless the number of configured servers. + +At all times, no more than one server can be set to "auto-connect" simultaneously. Setting a new one will effectively unset the previous one, if any. + +## Considered option + +* Auto-connect only of there's a single server configured. +* Allow to set the server as "auto-connect" during server creation, edition or import. +* Allow to set the server as "auto-connect" on a separated flow, where the full list of servers can be handled. + +## Decision outcome + +In order to make it more flexible, any server will be allowed to be set as "auto-connect", regardless the amount of configured servers. + +Auto-connect will be handled from the new "Manage servers" section. + +## Pros and Cons of the Options + +### Only one server + +* Good: + * Does not require extending models, and the logic to auto-connect is based on the amount of configured servers. +* Bad: + * It's not flexible enough. + * Makes the app behave differently depending on the amount of configured servers, making it confusing. + +### Auto-connect configured on existing creation/edition/import + +* Good: + * Does not require creating a new section to handle "auto-connect". +* Bad: + * Requires extending the server model with a new prop. + * It's much harder to ensure data consistency, as we need to ensure only one server is set to "auto-connect". + * On import, many servers might have been set to "auto-connect". The expected behavior there can be unclear. + +### Auto-connect configured on new section + +* Good: + * It's much easier to ensure data consistency. + * It's much more clear and predictable, as the UI shows which is the server configured as auto-connect. + * We have controls in a single place to set/unset auto connect on servers, allowing only the proper option based on current state for every server. +* Bad: + * Requires extending the server model with a new prop. + * Requires creating a new section to handle "auto-connect". diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 00000000..86ea34d6 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,5 @@ +# Architectural Decision Records + +Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome. + +* [2021-10-31 How to handle setting auto-connect on servers](2021-10-31-how-to-handle-setting-auto-connect-on-servers.md) diff --git a/src/app/App.tsx b/src/app/App.tsx index 0f43ab29..95279ee4 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -1,5 +1,6 @@ import { useEffect, FC } from 'react'; -import { Route, Switch } from 'react-router-dom'; +import { Route, RouteChildrenProps, Switch } from 'react-router-dom'; +import classNames from 'classnames'; import NotFound from '../common/NotFound'; import { ServersMap } from '../servers/data'; import { Settings } from '../settings/reducers/settings'; @@ -8,7 +9,7 @@ import { AppUpdateBanner } from '../common/AppUpdateBanner'; import { forceUpdate } from '../utils/helpers/sw'; import './App.scss'; -interface AppProps { +interface AppProps extends RouteChildrenProps { fetchServers: () => void; servers: ServersMap; settings: Settings; @@ -25,7 +26,9 @@ const App = ( Settings: FC, ManageServers: FC, ShlinkVersionsContainer: FC, -) => ({ fetchServers, servers, settings, appUpdated, resetAppUpdate }: AppProps) => { +) => ({ fetchServers, servers, settings, appUpdated, resetAppUpdate, location }: AppProps) => { + const isHome = location.pathname === '/'; + useEffect(() => { // On first load, try to fetch the remote servers if the list is empty if (Object.keys(servers).length === 0) { @@ -40,7 +43,7 @@ const App = (
-
+
diff --git a/src/app/services/provideServices.ts b/src/app/services/provideServices.ts index 50bb8f12..752c1cc3 100644 --- a/src/app/services/provideServices.ts +++ b/src/app/services/provideServices.ts @@ -1,9 +1,9 @@ -import Bottle from 'bottlejs'; +import Bottle, { Decorator } from 'bottlejs'; import { appUpdateAvailable, resetAppUpdate } from '../reducers/appUpdates'; import App from '../App'; import { ConnectDecorator } from '../../container/types'; -const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { +const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: Decorator) => { // Components bottle.serviceFactory( 'App', @@ -18,6 +18,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { 'ShlinkVersionsContainer', ); bottle.decorator('App', connect([ 'servers', 'settings', 'appUpdated' ], [ 'fetchServers', 'resetAppUpdate' ])); + bottle.decorator('App', withRouter); // Actions bottle.serviceFactory('appUpdateAvailable', () => appUpdateAvailable); diff --git a/src/common/Home.scss b/src/common/Home.scss index c8a00d35..e3bf4a88 100644 --- a/src/common/Home.scss +++ b/src/common/Home.scss @@ -4,6 +4,7 @@ .home { position: relative; padding-top: 15px; + width: 100%; @media (min-width: $mdMin) { padding-top: 0; diff --git a/src/container/index.ts b/src/container/index.ts index ce39f10e..1fbf7bd3 100644 --- a/src/container/index.ts +++ b/src/container/index.ts @@ -33,7 +33,7 @@ const connect: ConnectDecorator = (propsFromState: string[] | null, actionServic actionServiceNames.reduce(mapActionService, {}), ); -provideAppServices(bottle, connect); +provideAppServices(bottle, connect, withRouter); provideCommonServices(bottle, connect, withRouter); provideApiServices(bottle); provideShortUrlsServices(bottle, connect, withRouter); diff --git a/src/servers/data/index.ts b/src/servers/data/index.ts index 809a6636..a06c390e 100644 --- a/src/servers/data/index.ts +++ b/src/servers/data/index.ts @@ -1,3 +1,4 @@ +import { omit } from 'ramda'; import { SemVer } from '../../utils/helpers/version'; export interface ServerData { @@ -43,3 +44,6 @@ export const isNotFoundServer = (server: SelectedServer): server is NotFoundServ !!server?.hasOwnProperty('serverNotFound'); export const getServerId = (server: SelectedServer) => isServerWithId(server) ? server.id : ''; + +export const serverWithIdToServerData = (server: ServerWithId): ServerData => + omit([ 'id', 'autoConnect' ], server); diff --git a/src/servers/services/ServersExporter.ts b/src/servers/services/ServersExporter.ts index f230af67..08874cff 100644 --- a/src/servers/services/ServersExporter.ts +++ b/src/servers/services/ServersExporter.ts @@ -1,7 +1,7 @@ -import { dissoc, values } from 'ramda'; +import { values } from 'ramda'; import { CsvJson } from 'csvjson'; import LocalStorage from '../../utils/services/LocalStorage'; -import { ServersMap } from '../data'; +import { ServersMap, serverWithIdToServerData } from '../data'; import { saveCsv } from '../../utils/helpers/files'; const SERVERS_FILENAME = 'shlink-servers.csv'; @@ -14,7 +14,7 @@ export default class ServersExporter { ) {} public readonly exportServers = async () => { - const servers = values(this.storage.get('servers') ?? {}).map(dissoc('id')); + const servers = values(this.storage.get('servers') ?? {}).map(serverWithIdToServerData); try { const csv = this.csvjson.toCSV(servers, { headers: 'key' }); diff --git a/test/app/App.test.tsx b/test/app/App.test.tsx index 1638b6fc..a161a44e 100644 --- a/test/app/App.test.tsx +++ b/test/app/App.test.tsx @@ -1,6 +1,8 @@ import { shallow, ShallowWrapper } from 'enzyme'; import { Route } from 'react-router-dom'; import { Mock } from 'ts-mockery'; +import { match } from 'react-router'; +import { History, Location } from 'history'; import { Settings } from '../../src/settings/reducers/settings'; import appFactory from '../../src/app/App'; import { AppUpdateBanner } from '../../src/common/AppUpdateBanner'; @@ -9,19 +11,17 @@ describe('', () => { let wrapper: ShallowWrapper; const MainHeader = () => null; const ShlinkVersions = () => null; - - beforeEach(() => { - const App = appFactory( - MainHeader, - () => null, - () => null, - () => null, - () => null, - () => null, - () => null, - ShlinkVersions, - ); - + const App = appFactory( + MainHeader, + () => null, + () => null, + () => null, + () => null, + () => null, + () => null, + ShlinkVersions, + ); + const createWrapper = (pathname = '') => { wrapper = shallow( {}} @@ -29,18 +29,27 @@ describe('', () => { settings={Mock.all()} appUpdated={false} resetAppUpdate={() => {}} + match={Mock.all()} + history={Mock.all()} + location={Mock.of({ pathname })} />, ); - }); + + return wrapper; + }; + afterEach(() => wrapper.unmount()); - it('renders a header', () => expect(wrapper.find(MainHeader)).toHaveLength(1)); + it('renders children components', () => { + const wrapper = createWrapper(); - it('renders versions', () => expect(wrapper.find(ShlinkVersions)).toHaveLength(1)); - - it('renders an update banner', () => expect(wrapper.find(AppUpdateBanner)).toHaveLength(1)); + expect(wrapper.find(MainHeader)).toHaveLength(1); + expect(wrapper.find(ShlinkVersions)).toHaveLength(1); + expect(wrapper.find(AppUpdateBanner)).toHaveLength(1); + }); it('renders app main routes', () => { + const wrapper = createWrapper(); const routes = wrapper.find(Route); const expectedPaths = [ '/', @@ -57,4 +66,15 @@ describe('', () => { expect(routes.at(index).prop('path')).toEqual(path); }); }); + + it.each([ + [ '/foo', 'shlink-wrapper' ], + [ '/bar', 'shlink-wrapper' ], + [ '/', 'shlink-wrapper d-flex d-md-block align-items-center' ], + ])('renders expected classes on shlink-wrapper based on current pathname', (pathname, expectedClasses) => { + const wrapper = createWrapper(pathname); + const shlinkWrapper = wrapper.find('.shlink-wrapper'); + + expect(shlinkWrapper.prop('className')).toEqual(expectedClasses); + }); }); diff --git a/test/servers/services/ServersExporter.test.ts b/test/servers/services/ServersExporter.test.ts index fc990b1a..cfb69bad 100644 --- a/test/servers/services/ServersExporter.test.ts +++ b/test/servers/services/ServersExporter.test.ts @@ -10,10 +10,12 @@ describe('ServersExporter', () => { abc123: { id: 'abc123', name: 'foo', + autoConnect: true, }, def456: { id: 'def456', name: 'bar', + autoConnect: false, }, })), });