Skip to content

Commit

Permalink
Extract logic to determine if a list of servers contains duplicates
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Oct 31, 2024
1 parent 913264b commit 9134d07
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 45 deletions.
2 changes: 2 additions & 0 deletions scripts/docker/servers_from_env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ set -e

ME=$(basename $0)

# In order to allow people to pre-configure a server in their shlink-web-client instance via env vars, this function
# dumps a servers.json file based on the values provided via env vars
setup_single_shlink_server() {
[ -n "$SHLINK_SERVER_URL" ] || return 0
[ -n "$SHLINK_SERVER_API_KEY" ] || return 0
Expand Down
43 changes: 25 additions & 18 deletions src/servers/helpers/ImportServersBtn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { componentFactory, useDependencies } from '../../container/utils';
import type { ServerData, ServersMap } from '../data';
import type { ServersImporter } from '../services/ServersImporter';
import { DuplicatedServersModal } from './DuplicatedServersModal';
import { dedupServers } from './index';

export type ImportServersBtnProps = PropsWithChildren<{
onImport?: () => void;
Expand All @@ -26,9 +27,6 @@ type ImportServersBtnDeps = {
ServersImporter: ServersImporter
};

const serversInclude = (servers: ServerData[], { url, apiKey }: ServerData) =>
servers.some((server) => server.url === url && server.apiKey === apiKey);

const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBtnDeps> = ({
createServers,
servers,
Expand All @@ -43,44 +41,45 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
const [duplicatedServers, setDuplicatedServers] = useState<ServerData[]>([]);
const [isModalOpen,, showModal, hideModal] = useToggle();

const serversToCreate = useRef<ServerData[]>([]);
const importedServersRef = useRef<ServerData[]>([]);
const newServersRef = useRef<ServerData[]>([]);

const create = useCallback((serversData: ServerData[]) => {
createServers(serversData);
onImport();
}, [createServers, onImport]);
const onFile = useCallback(
async ({ target }: ChangeEvent<HTMLInputElement>) =>
serversImporter.importServersFromFile(target.files?.[0])
.then((newServers) => {
serversToCreate.current = newServers;
.then((importedServers) => {
const { duplicatedServers, newServers } = dedupServers(servers, importedServers);

const existingServers = Object.values(servers);
const dupServers = newServers.filter((server) => serversInclude(existingServers, server));
const hasDuplicatedServers = !!dupServers.length;
importedServersRef.current = importedServers;
newServersRef.current = newServers;

if (!hasDuplicatedServers) {
create(newServers);
if (duplicatedServers.length === 0) {
create(importedServers);
} else {
setDuplicatedServers(dupServers);
setDuplicatedServers(duplicatedServers);
showModal();
}
})
.then(() => {
// Reset input after processing file
// Reset file input after processing file
(target as { value: string | null }).value = null;
})
.catch(onImportError),
[create, onImportError, servers, serversImporter, showModal],
);

const createAllServers = useCallback(() => {
create(serversToCreate.current);
create(importedServersRef.current);
hideModal();
}, [create, hideModal, serversToCreate]);
}, [create, hideModal]);
const createNonDuplicatedServers = useCallback(() => {
create(serversToCreate.current.filter((server) => !serversInclude(duplicatedServers, server)));
create(newServersRef.current);
hideModal();
}, [create, duplicatedServers, hideModal]);
}, [create, hideModal]);

return (
<>
Expand All @@ -91,7 +90,15 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
You can create servers by importing a CSV file with <b>name</b>, <b>apiKey</b> and <b>url</b> columns.
</UncontrolledTooltip>

<input type="file" accept=".csv" className="d-none" ref={ref} onChange={onFile} aria-hidden />
<input
type="file"
accept=".csv"
className="d-none"
aria-hidden
ref={ref}
onChange={onFile}
data-testid="csv-file-input"
/>

<DuplicatedServersModal
isOpen={isModalOpen}
Expand Down
50 changes: 50 additions & 0 deletions src/servers/helpers/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { groupBy } from '@shlinkio/data-manipulation';
import type { ServerData, ServersMap, ServerWithId } from '../data';

/**
* Builds a potentially unique ID for a server, based on concatenating their name and the hostname of their domain, all
* in lowercase and replacing invalid URL characters with hyphens.
*/
function idForServer(server: ServerData): string {
const url = new URL(server.url);
return `${server.name} ${url.host}`.toLowerCase().replace(/[^a-zA-Z0-9-_.~]/g, '-');
}

export function serverWithId(server: ServerWithId | ServerData): ServerWithId {
if ('id' in server) {
return server;
}

const id = idForServer(server);
return { ...server, id };
}

export function serversListToMap(servers: ServerWithId[]): ServersMap {
return servers.reduce<ServersMap>(
(acc, server) => ({ ...acc, [server.id]: server }),
{},
);
}

const serversInclude = (serversList: ServerData[], { url, apiKey }: ServerData) =>
serversList.some((server) => server.url === url && server.apiKey === apiKey);

export type DedupServersResult = {
/** Servers which already exist in the reference list */
duplicatedServers: ServerData[];
/** Servers which are new based on a reference list */
newServers: ServerData[];
};

/**
* Given a list of new servers, checks which of them already exist in a servers map, and which don't
*/
export function dedupServers(servers: ServersMap, serversToAdd: ServerData[]): DedupServersResult {
const serversList = Object.values(servers);
const { duplicatedServers = [], newServers = [] } = groupBy(
serversToAdd,
(server) => serversInclude(serversList, server) ? 'duplicatedServers' : 'newServers',
);

return { duplicatedServers, newServers };
}
15 changes: 1 addition & 14 deletions src/servers/reducers/servers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { PayloadAction } from '@reduxjs/toolkit';
import { createSlice } from '@reduxjs/toolkit';
import { randomUUID } from '../../utils/utils';
import type { ServerData, ServersMap, ServerWithId } from '../data';
import { serversListToMap, serverWithId } from '../helpers';

interface EditServer {
serverId: string;
Expand All @@ -15,19 +15,6 @@ interface SetAutoConnect {

const initialState: ServersMap = {};

const serverWithId = (server: ServerWithId | ServerData): ServerWithId => {
if ('id' in server) {
return server;
}

return { ...server, id: randomUUID() };
};

const serversListToMap = (servers: ServerWithId[]): ServersMap => servers.reduce<ServersMap>(
(acc, server) => ({ ...acc, [server.id]: server }),
{},
);

export const { actions, reducer } = createSlice({
name: 'shlink/servers',
initialState,
Expand Down
25 changes: 12 additions & 13 deletions test/servers/helpers/ImportServersBtn.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fireEvent, screen, waitFor } from '@testing-library/react';
import { screen, waitFor } from '@testing-library/react';
import { fromPartial } from '@total-typescript/shoehorn';
import type { ServersMap, ServerWithId } from '../../../src/servers/data';
import type {
Expand All @@ -9,6 +9,7 @@ import { checkAccessibility } from '../../__helpers__/accessibility';
import { renderWithEvents } from '../../__helpers__/setUpTest';

describe('<ImportServersBtn />', () => {
const csvFile = new File([''], 'servers.csv', { type: 'text/csv' });
const onImportMock = vi.fn();
const createServersMock = vi.fn();
const importServersFromFile = vi.fn().mockResolvedValue([]);
Expand Down Expand Up @@ -54,14 +55,13 @@ describe('<ImportServersBtn />', () => {
});

it('imports servers when file input changes', async () => {
const { container } = setUp();
const input = container.querySelector('[type=file]');
const { user } = setUp();

const input = screen.getByTestId('csv-file-input');
await user.upload(input, csvFile);

if (input) {
fireEvent.change(input, { target: { files: [''] } });
}
expect(importServersFromFile).toHaveBeenCalledTimes(1);
await waitFor(() => expect(createServersMock).toHaveBeenCalledTimes(1));
expect(createServersMock).toHaveBeenCalledTimes(1);
});

it.each([
Expand All @@ -70,15 +70,14 @@ describe('<ImportServersBtn />', () => {
])('creates expected servers depending on selected option in modal', async (btnName, savesDuplicatedServers) => {
const existingServer = fromPartial<ServerWithId>({ id: 'abc', url: 'existingUrl', apiKey: 'existingApiKey' });
const newServer = fromPartial<ServerWithId>({ url: 'newUrl', apiKey: 'newApiKey' });
const { container, user } = setUp({}, { abc: existingServer });
const input = container.querySelector('[type=file]');
const { user } = setUp({}, { abc: existingServer });
const input = screen.getByTestId('csv-file-input');

importServersFromFile.mockResolvedValue([existingServer, newServer]);

expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
if (input) {
fireEvent.change(input, { target: { files: [''] } });
}
await waitFor(() => expect(screen.getByRole('dialog')).toBeInTheDocument());
await user.upload(input, csvFile);
expect(screen.getByRole('dialog')).toBeInTheDocument();
await user.click(screen.getByRole('button', { name: btnName }));

expect(createServersMock).toHaveBeenCalledWith(savesDuplicatedServers ? [existingServer, newServer] : [newServer]);
Expand Down

0 comments on commit 9134d07

Please sign in to comment.