Merge pull request #529 from shlinkio/develop

Release 3.4.1
This commit is contained in:
Alejandro Celaya 2021-11-20 10:11:57 +01:00 committed by GitHub
commit 47f5f47867
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 134 additions and 28 deletions

View file

@ -5,3 +5,4 @@
./test
./shlink-web-client.gif
./dist
./docs

View file

@ -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.

View file

@ -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".

5
docs/adr/README.md Normal file
View file

@ -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)

View file

@ -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 = (
<MainHeader />
<div className="app">
<div className="shlink-wrapper">
<div className={classNames('shlink-wrapper', { 'd-flex d-md-block align-items-center': isHome })}>
<Switch>
<Route exact path="/" component={Home} />
<Route exact path="/settings" component={Settings} />

View file

@ -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);

View file

@ -4,6 +4,7 @@
.home {
position: relative;
padding-top: 15px;
width: 100%;
@media (min-width: $mdMin) {
padding-top: 0;

View file

@ -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);

View file

@ -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<ServerWithId, 'id' | 'autoConnect'>([ 'id', 'autoConnect' ], server);

View file

@ -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<ServersMap>('servers') ?? {}).map(dissoc('id'));
const servers = values(this.storage.get<ServersMap>('servers') ?? {}).map(serverWithIdToServerData);
try {
const csv = this.csvjson.toCSV(servers, { headers: 'key' });

View file

@ -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('<App />', () => {
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(
<App
fetchServers={() => {}}
@ -29,18 +29,27 @@ describe('<App />', () => {
settings={Mock.all<Settings>()}
appUpdated={false}
resetAppUpdate={() => {}}
match={Mock.all<match>()}
history={Mock.all<History>()}
location={Mock.of<Location>({ 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('<App />', () => {
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);
});
});

View file

@ -10,10 +10,12 @@ describe('ServersExporter', () => {
abc123: {
id: 'abc123',
name: 'foo',
autoConnect: true,
},
def456: {
id: 'def456',
name: 'bar',
autoConnect: false,
},
})),
});